-
Notifications
You must be signed in to change notification settings - Fork 13.9k
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(dataset): handle missing database in migration #18948
fix(dataset): handle missing database in migration #18948
Conversation
Codecov Report
@@ Coverage Diff @@
## master #18948 +/- ##
==========================================
- Coverage 66.39% 66.31% -0.08%
==========================================
Files 1640 1640
Lines 63512 63512
Branches 6417 6417
==========================================
- Hits 42170 42121 -49
- Misses 19681 19730 +49
Partials 1661 1661
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
if database: | ||
engine = database.get_sqla_engine(schema=target.schema) | ||
conditional_quote = ( | ||
engine.dialect.identifier_preparer.quote if engine else lambda x: x |
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.
I think we should just skip the dataset if it has no database... it seems that this is something that shouldn't happen, and a dataset without an associated database is useless, no? Or am I missing something?
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. Thanks for this fix.
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.
If there's no legit reason for a dataset without a database let's just return
early in the function when that happens.
(cherry picked from commit 2bacedd)
SUMMARY
The recent PR #18935 added a migration that fails if a dataset is referencing a database that has been removed. This changes the migration logic to return early if the database is missing.
BEFORE
When performing a migration on metadata containing datasets that reference a missing database, the following happens:
AFTER
After the change the migration completes successfully:
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION