-
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
Filtering out SQLLab views out of table list view by default #4746
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4746 +/- ##
==========================================
+ Coverage 72.64% 72.66% +0.01%
==========================================
Files 205 205
Lines 15402 15413 +11
Branches 1183 1183
==========================================
+ Hits 11189 11200 +11
Misses 4210 4210
Partials 3 3
Continue to review full report at Codecov.
|
@@ -213,8 +216,10 @@ class TableModelView(DatasourceModelView, DeleteMixin, YamlExportMixin): # noqa | |||
"Whether to populate the filter's dropdown in the explore " | |||
"view's filter section with a list of distinct values fetched " | |||
'from the backend on the fly'), | |||
'is_sqllab_view': _( |
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.
Is there anyway to infer this rather than it be a form option?
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 could remove it from the edit_columns
and have it only in the show_columns
if you think it's better
# Use Slice class defined here instead of models.Slice | ||
for tbl in session.query(Table).all(): | ||
if tbl.sql: | ||
tbl.is_sqllab_view = True |
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 guess the presence of custom SQL doesn't always imply that the SQL corresponds to a SQL Lab view.
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.
Right, I'm not sure how else to identify it. It's the best proxy we have. Note that this does not change any behavior other than being filtered out by default in the Table list view, but the filter can be removed anyways, so I didn't think it had to be perfect. Goal here is just to hide some of the clutter this creates.
if tbl.sql: | ||
tbl.is_sqllab_view = True | ||
session.merge(tbl) | ||
session.commit() |
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 the session.commit()
could be defined outside the for loop.
This adds a `is_sqllab_view` flag in the "tables" table, and makes the filters those out in the Tables list view. The filter showing on top may be a bit confusing, but certainly less than seeing lots of user generated views.
3d83a1b
to
1faa31f
Compare
Addressed comments, mergin |
…4746) * Filtering out SQLLab views out of table list view by default This adds a `is_sqllab_view` flag in the "tables" table, and makes the filters those out in the Tables list view. The filter showing on top may be a bit confusing, but certainly less than seeing lots of user generated views. * flake * Addressing comments
…4746) * Filtering out SQLLab views out of table list view by default This adds a `is_sqllab_view` flag in the "tables" table, and makes the filters those out in the Tables list view. The filter showing on top may be a bit confusing, but certainly less than seeing lots of user generated views. * flake * Addressing comments
@mistercrunch I think this change should be added to UPDATING.md. |
…4746) * Filtering out SQLLab views out of table list view by default This adds a `is_sqllab_view` flag in the "tables" table, and makes the filters those out in the Tables list view. The filter showing on top may be a bit confusing, but certainly less than seeing lots of user generated views. * flake * Addressing comments
This adds a
is_sqllab_view
flag in the "tables" table, and makes thefilters those out in the Tables list view.
The filter showing on top may be a bit confusing, but certainly less
than seeing lots of user generated views.
It fixes this:
The default link to "Tables" is a bit confusing as we show the filter by default. I tried other options like using FAB's
base_filters
but it prevents the SQL Lab view from being edited / visible in any way.