-
Notifications
You must be signed in to change notification settings - Fork 13.8k
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: None dataset and schema permissions #20108
fix: None dataset and schema permissions #20108
Conversation
Codecov Report
@@ Coverage Diff @@
## master #20108 +/- ##
==========================================
- Coverage 66.47% 66.46% -0.01%
==========================================
Files 1721 1721
Lines 64477 64485 +8
Branches 6795 6795
==========================================
+ Hits 42858 42862 +4
- Misses 19891 19895 +4
Partials 1728 1728
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
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, this is a really welcome improvement!
* fix: None dataset and schema permissions * fix pylint * add migration and test * fix migration
SUMMARY
Currently load examples creates duplicate schema and dataset permissions like:
This is due to the fact that a SqlaTable can be created without an associated database triggering SQLAlchemy listen
set_perm
with a None database.This is a simple/patch fix, we should revisit the entire logic around creating permissions for datasets, databases and schemas since it's possible to have orphaned permissions also when (for example) a database name is changed.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION