Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(fivetran_sdk): Create a separate folder for V1 examples #79

Merged

Conversation

fivetran-satvikpatil
Copy link
Contributor

@fivetran-satvikpatil fivetran-satvikpatil commented Dec 30, 2024

Closes T-859009

Description

  • We will be keeping both proto version examples in the repo, so there will be two folders for two different version
  • In this PR, created a separate PR for v1 examples

Testing

Source

  • Golang
Screenshot 2024-12-30 at 1 11 31 PM
  • Nodejs
Screenshot 2024-12-30 at 1 09 26 PM
  • Python
Screenshot 2024-12-30 at 1 08 44 PM
  • Java
Screenshot 2024-12-30 at 1 08 13 PM

Destination

  • Python
Screenshot 2024-12-30 at 1 17 08 PM
  • Java
Screenshot 2024-12-30 at 1 15 51 PM

Copy link

height bot commented Dec 30, 2024

This pull request has been linked to and will mark 1 task as "Done" when merged:

💡Tip: You can link multiple Height tasks to a pull request.

@fivetran-satvikpatil fivetran-satvikpatil changed the base branch from main to partner-sdk/v2-implementation December 30, 2024 07:28
@fivetran-satvikpatil fivetran-satvikpatil marked this pull request as ready for review December 30, 2024 07:48
Copy link
Collaborator

@varundhall varundhall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you please update the github workflow, to ensure these examples are build/tested properly.

Copy link
Contributor

@fivetran-rishabhghosh fivetran-rishabhghosh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look good.
Just one question:

  • Once this PR is merged, I believe there is no change with testers and thus customers would not be affected?

@fivetran-satvikpatil
Copy link
Contributor Author

@fivetran-rishabhghosh Yes, there are no changes in testers for now. The examples are working fine.
Partners might be affected if they are pulling the latest proto version from the repo while building the executable.
But that is something we need to make sure and inform while building executables.

This is not a problem for now, as I am not merging these changes in the main branch. We will be doing it later.

@fivetran-satvikpatil fivetran-satvikpatil changed the base branch from partner-sdk/v2-implementation to main December 30, 2024 11:11
@fivetran-rishabhghosh
Copy link
Contributor

fivetran-rishabhghosh commented Dec 30, 2024

@fivetran-satvikpatil @varundhall It is better we merge this to main branch
There would not be any problem. Let's not complicate

Copy link
Collaborator

@varundhall varundhall left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The CI job is failing, please check it.

@fivetran-satvikpatil
Copy link
Contributor Author

fivetran-satvikpatil commented Jan 2, 2025

@varundhall I fixed argument issue, but now it is failing with another issue. Created a ticket to fix it: https://fivetran.height.app/T-860002

@fivetran-satvikpatil fivetran-satvikpatil merged commit 09a813c into main Jan 2, 2025
1 check failed
@fivetran-satvikpatil fivetran-satvikpatil deleted the satvik/T-859009-restructuring-for-adding-v2 branch January 2, 2025 12:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants