-
Notifications
You must be signed in to change notification settings - Fork 14k
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
[migration] make names non-nullable #8371
Conversation
8bd6ff6
to
6499338
Compare
superset/migrations/versions/b6fa807eac07_make_names_non_nullable.py
Outdated
Show resolved
Hide resolved
superset/migrations/versions/b6fa807eac07_make_names_non_nullable.py
Outdated
Show resolved
Hide resolved
6499338
to
79ee41d
Compare
looks like ci is broken |
79ee41d
to
ce0f2c6
Compare
ce0f2c6
to
eadad16
Compare
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’m still somewhat perplexed as to why this was working before, i.e., how one could delete a cluster when there were associated datasources.
I think your logic seems sound. The Cypress tests were failing so I restarted them (they can be flakey at times).
eadad16
to
ce0b8cf
Compare
Codecov Report
@@ Coverage Diff @@
## master #8371 +/- ##
==========================================
- Coverage 67.57% 60.55% -7.03%
==========================================
Files 448 332 -116
Lines 22527 10254 -12273
Branches 2364 2364
==========================================
- Hits 15222 6209 -9013
+ Misses 7167 3907 -3260
Partials 138 138 Continue to review full report at Codecov.
|
CATEGORY
Choose one
SUMMARY
Another try for #8332, which made
dbs.database_name
,clusters.cluster_name
, andtables.table_name
non-null to prevent__repr__
from ever returningNone
.Tweaked to also make
datasources.cluster_name
non-nullable. It has a foreign key constraint withclusters.cluster_name
and was breaking deploys.TEST PLAN
Ran upgrade and downgrade, confirmed that the database schema looks as expected after each action.
Deployed. Confirmed does not break deploy.
ADDITIONAL INFORMATION
REVIEWERS
@etr2460 @john-bodley