-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
⚠️ Retire destination-bigquery-denormalized
#28488
⚠️ Retire destination-bigquery-denormalized
#28488
Conversation
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,
|
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.
🍄 ☁️
(assuming the metadata changes work the way we want)
Update - for the period until Nov1, we are just going to call this a breaking change and publish a V2 is a no-op. Then we can delete the connector after Nov 1 |
destination-bigquery-denormalized
destination-bigquery-denormalized
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.
Thumbs up on the metadata side!
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... though getting tests to green might be a challenge, given the state of the bq-denormalized codebase
it might actually be easier to just manually copy the docker image to 2.0.0 instead of publishing a new build, and then do any metadata operations to activate the breaking changes stuff? That feels safer too - I definitely wasn't trying to keep bigquery-denormalized in good shape when publishing dv2, so there might be real test failures in the current codebase.
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.
This will be a little weird that it will still tell the user to upgrade by the deadline. If they upgrade, their connections will still sync, with the same code as before. Maybe that's what you meant in the migration guide?
docs/integrations/destinations/bigquery-denormalized-migrations.md
Outdated
Show resolved
Hide resolved
breakingChanges: | ||
2.0.0: | ||
message: "`destination-bigquery-denormalized` is being retired in favor of `destination-bigquery`, and is no longer maintained. Please switch to destination-bigquery, which will produce similar tables and contains many improvements. Learn more [here](https://docs.airbyte.com/release_notes/upgrading_to_destinations_v2/)." | ||
upgradeDeadline: "2023-11-01" |
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.
Once we have the sync pauser, after this date there should be no more running syncs on this destination - at which time (or maybe a little after, to give them the kick in the butt to move over to the new one once the syncs stop running) we can tombstone the connector (which tombstones their actors/connections) 👍🏻
Basically I'm wondering if the end goal of this is "all syncs are paused, and then we can delete them" or "we let them keep vibing but shrug if they have any problems" |
…s.md Co-authored-by: Ella Rohm-Ensing <erohmensing@gmail.com>
I added one final change in e758357 to override the dockerImageTag in both cloud and oss to continue run I'll will be doing step 2 later this week, where we remove both the code from this repo, and set the registries to false, so no new connections can be made |
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.
does the registry override mean that we won't get the breaking change notification? (because cloud will see the latest version as 1.5.3)
otherwise
Dang! Good point. Ok, reverted that change in 38cc904 - I'll do the manual docker image tagging to duplicate 1.5.3 to 2.0.0 first before merging this in. |
destination-bigquery-denormalized test report (commit
|
Step | Result |
---|---|
Java Connector Unit Tests | ✅ |
Build connector tar | ✅ |
Build destination-bigquery-denormalized docker image for platform linux/x86_64 | ✅ |
Java Connector Integration Tests | ❌ |
Validate airbyte-integrations/connectors/destination-bigquery-denormalized/metadata.yaml | ✅ |
Connector version semver check | ✅ |
Connector version increment check | ✅ |
QA checks | ✅ |
☁️ View runs for commit in Dagger Cloud
Please note that tests are only run on PR ready for review. Please set your PR to draft mode to not flood the CI engine and upstream service on following commits.
You can run the same pipeline locally on this branch with the airbyte-ci tool with the following command
airbyte-ci connectors --name=destination-bigquery-denormalized test
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.
womp womp. sounds like a plan
I have manually created the docker tag
It looks right on docker hub because the SHAs match v1.5.3 |
/approve-and-merge reason="destination-bigquery-denormalized is going away. This PR adds a breaking change warning" |
Closes #28449
Breaking Change Playbook - https://docs.google.com/document/d/1kJjEsc5UrYxVIkkS4eaEFIf9gCoHIPKlVMqADPaqiVY/edit
The destination was created in 2021 here #4176