-
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
feat: Custom superset_config.py + secret envs #13096
Conversation
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.
this is a major upgrade to the flexibility of managing multiple supersets at scale
left one comment
# from flask_appbuilder.security.manager import AUTH_DB | ||
# AUTH_TYPE = AUTH_OAUTH | ||
|
||
# OAUTH_PROVIDERS = [ | ||
# { | ||
# "name": "google", | ||
# "icon": "fa-google", | ||
# "token_key": "access_token", | ||
# "remote_app": { | ||
# "client_id": os.environ.get("GOOGLE_KEY"), | ||
# "client_secret": os.environ.get("GOOGLE_SECRET"), | ||
# "api_base_url": "https://www.googleapis.com/oauth2/v2/", | ||
# "client_kwargs": {"scope": "email profile"}, | ||
# "request_token_url": None, | ||
# "access_token_url": "https://accounts.google.com/o/oauth2/token", | ||
# "authorize_url": "https://accounts.google.com/o/oauth2/auth", | ||
# }, | ||
# } |
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.
Can this be injected from a separate file called enable_oauth_config.py?
by that, we can upgrade configOverrides
solution with multiple config modules
that can be independent separately and can be mixed with other modules seamlessly
we could have a config_modules folder from which all different config will be loaded from
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.
Yes, I think this is indeed probably a better approach... I went for appending to the existing file because that was the easiest for me, as I'm not 100% confident with my python / FAB skills and didn't want to break things...
I see that /app/pythonpath
already had other files:
superset_bootstrap.sh
superset_config.py
superset_init.sh
But I suppose it's safe to simply create those separate config modules in the same directory?
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.
Sorry I just realize you meant loading from a separate file and not into a separate file... :)
I'm not sure if there's a really convenient way to do that in the chart definition itself... it IS possible to read files but I seem to remember this comes with a few limitations...
At install time however, I think something like helm upgrade --set-file configOverrides.enable_oauth_config=oauth.py
would work - but AFAIK not with a full directory...
* master: (30 commits) refactor(native-filters): decouple params from filter config modal (first phase) (apache#13021) fix(native-filters): set currentValue null when empty (apache#13000) Custom superset_config.py + secret envs (apache#13096) Update http error code from 400 to 403 (apache#13061) feat(native-filters): add storybook entry for select filter (apache#13005) feat(native-filters): Time native filter (apache#12992) Force pod restart on config changes (apache#13056) feat(cross-filters): add cross filters (apache#12662) fix(explore): Enable selecting an option not included in suggestions (apache#13029) Improves RTL configuration (apache#13079) Added a note about the ! prefix for breaking changes to CONTRIBUTING.md (apache#13083) chore: lock down npm to v6 (apache#13069) fix: API tests, make them possible to run independently again (apache#13076) fix: add config to disable dataset ownership on the old api (apache#13051) add required * indicator to message content/notif method (apache#12931) fix: Retroactively add granularity param to charts (apache#12960) fix(ci): multiline regex in change detection (apache#13075) feat(style): hide dashboard header by url parameter (apache#12918) fix(explore): pie chart label bugs (apache#13052) fix: Disabled state button transition time (apache#13008) ...
SUMMARY
This update to the Helm chart provides support for:
superset_config.py
TEST PLAN
extraSecretEnv
configOverrides
Sample
values.yaml
: