-
Notifications
You must be signed in to change notification settings - Fork 14.3k
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
fix: Update downgrade path for migration to remove sl_tables #28838
fix: Update downgrade path for migration to remove sl_tables #28838
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #28838 +/- ##
===========================================
+ Coverage 60.48% 77.51% +17.02%
===========================================
Files 1931 518 -1413
Lines 76236 37435 -38801
Branches 8568 0 -8568
===========================================
- Hits 46114 29019 -17095
+ Misses 28017 8416 -19601
+ Partials 2105 0 -2105
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
Oops my bad, thanks for fixing this. I'm pretty sure I tested the downgrade at some point, but maybe I changed something after (?).
I'm thinking it'd be a good thing to add a CI step that tests downgrades.
SUMMARY
In #28704 we removed
sl_tables
however, in the downgrade path there are dependencies on othersl_tables
that cause the downgrade to fail.sl_datasets
referenced bysl_dataset_users
andsl_dataset_tables
sl_tables
referenced bysl_dataset_tables
andsl_table_columns
sl_columns
referenced bysl_table_columns
andsl_dataset_columns
so the new creation order is
sl_datasets
sl_tables
sl_dataset_columns
sl_dataset_users
sl_dataset_tables
sl_table_columns
sl_dataset_columns
Also normalized cases when creating columns since
sa.DATETIME()
was creating an issue butsa.DateTime()
would create the tables without any errors.BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION