-
Notifications
You must be signed in to change notification settings - Fork 14.1k
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(viz-gallery): respect denylist in viz gallery #22658
Conversation
Codecov Report
@@ Coverage Diff @@
## master #22658 +/- ##
==========================================
- Coverage 66.35% 65.59% -0.77%
==========================================
Files 1869 1869
Lines 71582 71582
Branches 7814 7814
==========================================
- Hits 47497 46952 -545
- Misses 22044 22589 +545
Partials 2041 2041
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
95a464d
to
13649fc
Compare
config = bootstrapData?.common?.conf; | ||
} else { | ||
config = {}; | ||
logging.warn('asyncEvent: app config data not found'); |
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.
Do we want to remove that log?
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.
Well, the weird thing is, I don't really see any situation where we would ever hit a page that doesn't have bootstrap data on it. This is related to GAQ, and GAQ is definitely always called from some place where there's bootstrap data. So I think it should be safe to remove (incidentally, I'll probably be working on GAQ soon, so I can address this later if it's really needed).
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.
1 tiny nit, LGTM
cb537a2
to
33a7390
Compare
SUMMARY
The config parameter
VIZ_TYPE_DENYLIST
should make it possible to omit certain viz plugins:superset/superset/config.py
Lines 777 to 783 in 159dcd7
However, currently the viz gallery displays all chart types from
MainPresets.js
despite adding specific charts to the denylist.To test, add the following to your `superset_config.py:
AFTER
With the changes, the big number and pie charts are all removed:
BEFORE
Currently they are all displayed:
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION