-
Notifications
You must be signed in to change notification settings - Fork 14.4k
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: alembic's 'superset db migrate' fails with CompileError #27846
Conversation
When running on the latest `master` running a simple `superset db migrate` to generate a new database migration fails with error ``` sqlalchemy.exc.CompileError: PostgreSQL ENUM type requires a name. ``` This addresses the issue by giving a name to that particular enum. From my understanding, this particular enum wasn't created in a given migration script, so it may not be enforced at the database level, probably by choice/design. Having the name here simply allows for alembic's migrate subcommand to run.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #27846 +/- ##
==========================================
+ Coverage 67.41% 69.83% +2.42%
==========================================
Files 1920 1920
Lines 75242 75242
Branches 8423 8423
==========================================
+ Hits 50724 52546 +1822
+ Misses 22457 20635 -1822
Partials 2061 2061
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.
implemented as a varchar here: https://github.com/apache/superset/blob/master/superset/migrations/versions/2020-09-15_18-22_e5ef6828ac4e_add_rls_filter_type_and_grouping_key.py
probably by design
Enum( | ||
*[filter_type.value for filter_type in utils.RowLevelSecurityFilterType], | ||
name="filter_type_enum", | ||
), |
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.
implemented here as a varchar: https://github.com/apache/superset/blob/master/superset/migrations/versions/2020-09-15_18-22_e5ef6828ac4e_add_rls_filter_type_and_grouping_key.py
probably by choice
@mistercrunch why is this an issue with Docker but not with global CI? |
@john-bodley this was an issue with Also note that auto-migrate has picks up the diff between the declared model and what's in the database post applying all migrations, and currently there are quite a bit of discrepancies that could be good to address. |
When running on the latest
master
running a simple
superset db migrate
withindocker-compose exec superset bash
to generate a newdatabase migration fails with error
This PR addresses the issue by giving a name to that particular enum.
From my understanding, this particular enum wasn't created in
a given migration script, so it may not be enforced at the database
level, probably by choice/design. Having the name here simply
allows for alembic's migrate subcommand to run.