codeant-ai-for-open-source[bot] commented on code in PR #37523:
URL: https://github.com/apache/superset/pull/37523#discussion_r2752961310
##########
superset/app.py:
##########
@@ -43,64 +43,84 @@
logger = logging.getLogger(__name__)
-
def create_app(
superset_config_module: Optional[str] = None,
superset_app_root: Optional[str] = None,
) -> Flask:
app = SupersetApp(__name__)
try:
- # Allow user to override our config completely
- config_module = superset_config_module or os.environ.get(
- "SUPERSET_CONFIG", "superset.config"
- )
- app.config.from_object(config_module)
-
- # Allow application to sit on a non-root path
- # *Please be advised that this feature is in BETA.*
- app_root = cast(
- str, superset_app_root or os.environ.get("SUPERSET_APP_ROOT", "/")
- )
- if app_root != "/":
- app.wsgi_app = AppRootMiddleware(app.wsgi_app, app_root)
- # If not set, manually configure options that depend on the
- # value of app_root so things work out of the box
- if not app.config["STATIC_ASSETS_PREFIX"]:
- app.config["STATIC_ASSETS_PREFIX"] = app_root
- # Prefix APP_ICON path with subdirectory root for subdirectory
deployments
- if (
- app.config.get("APP_ICON", "").startswith("/static/")
- and app_root != "/"
- ):
- app.config["APP_ICON"] = f"{app_root}{app.config['APP_ICON']}"
- # Also update theme tokens for subdirectory deployments
- for theme_key in ("THEME_DEFAULT", "THEME_DARK"):
- theme = app.config[theme_key]
- token = theme.get("token", {})
- # Update brandLogoUrl if it points to /static/
- if token.get("brandLogoUrl", "").startswith("/static/"):
- token["brandLogoUrl"] =
f"{app_root}{token['brandLogoUrl']}"
- # Update brandLogoHref if it's the default "/"
- if token.get("brandLogoHref") == "/":
- token["brandLogoHref"] = app_root
- if app.config["APPLICATION_ROOT"] == "/":
- app.config["APPLICATION_ROOT"] = app_root
-
- app_initializer = app.config.get("APP_INITIALIZER",
SupersetAppInitializer)(app)
- app_initializer.init_app()
-
- # Set up LOCAL_EXTENSIONS file watcher when in debug mode
- if app.debug:
- start_local_extensions_watcher_thread(app)
-
+ _configure_app(app, superset_config_module, superset_app_root)
+ _initialize_app(app)
+ _setup_debug_features(app)
return app
-
# Make sure that bootstrap errors ALWAYS get logged
except Exception:
logger.exception("Failed to create app")
raise
+def _configure_app(
+ app: Flask,
+ superset_config_module: Optional[str],
+ superset_app_root: Optional[str],
+) -> None:
+ # Allow user to override our config completely
+ config_module = superset_config_module or os.environ.get(
+ "SUPERSET_CONFIG", "superset.config"
+ )
+ app.config.from_object(config_module)
+
+ # Allow application to sit on a non-root path
+ # *Please be advised that this feature is in BETA.*
+ app_root = cast(
+ str, superset_app_root or os.environ.get("SUPERSET_APP_ROOT", "/")
+ )
+
+ if app_root != "/":
+ _configure_subdirectory_deployment(app, app_root)
+
+def _configure_subdirectory_deployment(app: Flask, app_root: str) -> None:
+ app.wsgi_app = AppRootMiddleware(app.wsgi_app, app_root)
+ # If not set, manually configure options that depend on the
+ # value of app_root so things work out of the box
+ if not app.config["STATIC_ASSETS_PREFIX"]:
+ app.config["STATIC_ASSETS_PREFIX"] = app_root
+
+ _update_app_icon_for_subdirectory(app, app_root)
+ _update_theme_tokens_for_subdirectory(app, app_root)
+
+ if app.config["APPLICATION_ROOT"] == "/":
+ app.config["APPLICATION_ROOT"] = app_root
+
+def _update_app_icon_for_subdirectory(app: Flask, app_root: str) -> None:
+ # Prefix APP_ICON path with subdirectory root for subdirectory deployments
+ app_icon = app.config.get("APP_ICON", "")
+ if app_icon.startswith("/static/"):
+ app.config["APP_ICON"] = f"{app_root}{app_icon}"
+
+def _update_theme_tokens_for_subdirectory(app: Flask, app_root: str) -> None:
+ # Prefix theme tokens for subdirectory deployments
+ for theme_key in ("THEME_DEFAULT", "THEME_DARK"):
+ theme = app.config[theme_key]
Review Comment:
**Suggestion:** Directly indexing theme configuration entries assumes that
"THEME_DEFAULT" and "THEME_DARK" are always defined, which can cause a KeyError
and crash initialization for fully custom configuration modules that omit these
keys. [possible bug]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ App initialization crashes for some custom configs.
- ⚠️ Theme token adjustments don't run for missing themes.
```
</details>
```suggestion
theme = app.config.get(theme_key)
if not isinstance(theme, dict):
continue
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Start Superset with create_app() in superset/app.py so _configure_app
runs (see
superset/app.py:62-80).
2. Configure a non-root deployment (SUPERSET_APP_ROOT=/analytics) causing
_configure_subdirectory_deployment to run (superset/app.py:82-93) which calls
_update_theme_tokens_for_subdirectory.
3. Provide a fully custom configuration module loaded by
app.config.from_object that omits
THEME_DEFAULT and/or THEME_DARK keys.
4. When _update_theme_tokens_for_subdirectory executes at
superset/app.py:101-114, the
code at superset/app.py:103-105 indexes app.config[theme_key] and raises
KeyError. Observe
initialization crash with traceback referencing superset/app.py:104.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/app.py
**Line:** 104:104
**Comment:**
*Possible Bug: Directly indexing theme configuration entries assumes
that "THEME_DEFAULT" and "THEME_DARK" are always defined, which can cause a
KeyError and crash initialization for fully custom configuration modules that
omit these keys.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
