[GitHub] [incubator-superset] mistercrunch commented on a change in pull request #8006: Local config no longer fails to import silently

[GitBox <gi...@apache.org>]

mistercrunch commented on a change in pull request #8006: Local config no longer fails to import silently
URL: https://github.com/apache/incubator-superset/pull/8006#discussion_r311849578
 
 

 ##########
 File path: superset/config.py
 ##########
 @@ -622,29 +624,31 @@ class CeleryConfig(object):
 # SQLALCHEMY_DATABASE_URI by default if set to `None`
 SQLALCHEMY_EXAMPLES_URI = None
 
-try:
-    if CONFIG_PATH_ENV_VAR in os.environ:
-        # Explicitly import config module that is not in pythonpath; useful
-        # for case where app is being executed via pex.
-        print(
-            "Loaded your LOCAL configuration at [{}]".format(
-                os.environ[CONFIG_PATH_ENV_VAR]
-            )
-        )
+if CONFIG_PATH_ENV_VAR in os.environ:
+    # Explicitly import config module that is not necessarily in pythonpath; useful
+    # for case where app is being executed via pex.
+    try:
+        cfg_path = os.environ[CONFIG_PATH_ENV_VAR]
         module = sys.modules[__name__]
-        override_conf = imp.load_source(
-            "superset_config", os.environ[CONFIG_PATH_ENV_VAR]
-        )
+        override_conf = imp.load_source("superset_config", cfg_path)
         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))
+    except Exception:
+        logging.exception(
+            "Failed to import config for {}={}".format(CONFIG_PATH_ENV_VAR, cfg_path)
+        )
+        raise
+elif importlib.util.find_spec("superset_config"):
+    try:
         from superset_config import *  # noqa
         import superset_config
 
         print(
             "Loaded your LOCAL configuration at [{}]".format(superset_config.__file__)
         )
-except ImportError:
-    pass
+    except Exception:
+        logging.exception("Failed to import local superset_confg")
+        raise
 
 Review comment:
   Oh yes can confirm that before we just were trying not to raise if not found. But if found the module and failed the import we totally should raise!

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org