-
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(migration): handle permalink edge cases correctly #23980
Conversation
Codecov Report
@@ Coverage Diff @@
## master #23980 +/- ##
==========================================
- Coverage 68.18% 67.51% -0.68%
==========================================
Files 1941 1941
Lines 75287 75287
Branches 8177 8177
==========================================
- Hits 51332 50827 -505
- Misses 21860 22365 +505
Partials 2095 2095
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 38 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
7c0cd58
to
8e9f9f5
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.
Thank you for the fix @villebro. I tested the migration here at Airbnb and it executed successfully.
Awesome, thanks for testing @michael-s-molina ! |
(cherry picked from commit 7a41170)
🏷️ preset:2023.19 |
SUMMARY
This is a continuation of #23888 and addresses two issues that were identified after this PR was merged:
str
(apparently my devenv didn't have a single Explore permalink when I ran the migration 🙁). For this reason we add a check tofind_class
to make suresuperset.utils.core.DatasourceType
is a valid class. Note that this problem will now go away, as the JSON codec ensures that the Enum becomes a simple string in the future. In addition and to be more explicit, we castdatasourceType
in the Explore payload of the create command to astr
which is the correct type based on theExplorePermalinkValue
type:superset/superset/explore/permalink/types.py
Lines 25 to 34 in d96b72d
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
7e67aecbf3f1
, checkout master prior tof1fa1a733d247ec4793c17b69c4eb40c631c5080
and create an Explore permalinksuperset db upgrade
and notice the errorsuperset db upgrade
and see the migration completeADDITIONAL INFORMATION