-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Source Faker: Declare primary keys #34644
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
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
Before Merging a Connector Pull RequestWow! What a great pull request you have here! 🎉 To merge this PR, ensure the following has been done/considered for each connector added or updated:
If the checklist is complete, but the CI check is failing,
|
@evantahler - You are the latest contributor to this connector and I wanted to ask your approval, in case there are any tests that would be negatively impacted by this change. If this does create a problem, I may need to find or build a different test connector. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Totally fine with this change. But, connectors are allowed to have user-defined PKs. Is there a use case where that doesn't work in airbyte-lib?
Thanks, @evantahler ! I did consider that... it's just not functionality we've exposed in the API or scoped out as of yet. As of now, AirbyteLib will just use whatever primary gets the source defines (if any). |
This declares the 'id' columns to be primary keys on all three streams.
This was driven by a need to leverage this test connector in: