-
Notifications
You must be signed in to change notification settings - Fork 13.6k
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
Local config no longer fails to import silently #8006
Conversation
superset/config.py
Outdated
for key in dir(override_conf): | ||
if key.isupper(): | ||
setattr(module, key, getattr(override_conf, key)) | ||
|
||
else: | ||
print("Loaded your LOCAL configuration at [{}]".format(cfg_path)) |
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.
Nit: can you update this to use f-strings? Superset no longer supports Python < 3.6.
superset/config.py
Outdated
print("Loaded your LOCAL configuration at [{}]".format(cfg_path)) | ||
except Exception: | ||
logging.exception( | ||
"Failed to import config for {}={}".format(CONFIG_PATH_ENV_VAR, cfg_path) |
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.
Nit: same here, using f-strings.
superset/config.py
Outdated
except ImportError: | ||
pass | ||
except Exception: | ||
logging.exception("Failed to import local superset_confg") |
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.
Typo: "superset_config"
closes #8004 |
@betodealmeida @mistercrunch et al... can someone kick the CI, it looks like some sporadic, unrelated issues caused it to fail. |
CATEGORY
Choose one
SUMMARY
Previously, local superset_config or configured (via env) file could fail to import and superset would swallow the error and continue to run. Now, if user has provided a path or a superset_config.py and it fails to import, the exception will be clearly logged and re-raised causing superset to stop immediately.
TEST PLAN
Manually tested with both forms of config (env and superset_config), with and without various errors.
ADDITIONAL INFORMATION
REVIEWERS
@betodealmeida @mistercrunch @john-bodley