aminghadersohi commented on code in PR #38562:
URL: https://github.com/apache/superset/pull/38562#discussion_r2932175771
##########
superset/mcp_service/flask_singleton.py:
##########
@@ -52,62 +51,45 @@
logger.info("Reusing existing Flask app from app context for MCP
service")
# Use _get_current_object() to get the actual Flask app, not the
LocalProxy
app = current_app._get_current_object()
+ elif appbuilder_initialized:
+ # appbuilder is initialized but we have no app context. Calling
+ # create_app() here would invoke appbuilder.init_app() a second
+ # time with a *different* Flask app, overwriting shared internal
+ # state (views, security manager, etc.). Fail loudly instead of
+ # silently corrupting the singleton.
+ raise RuntimeError(
+ "appbuilder is already initialized but no Flask app context is "
+ "available. Cannot call create_app() as it would re-initialize "
+ "appbuilder with a different Flask app instance."
+ )
Review Comment:
Not a valid concern. The bot is misunderstanding the execution model here.
The `RuntimeError` is raised in **module-level code** during import, not
inside `get_flask_app()`. The function `get_flask_app()` (line 117-124) simply
returns the module-level `app` variable — it never re-evaluates the branching
logic.
The three branches execute once at import time:
1. `appbuilder_initialized and has_app_context()` — Reuses existing app
(normal in-process Superset startup)
2. `appbuilder_initialized and not has_app_context()` — **New RuntimeError**
(prevents silent singleton corruption)
3. `not appbuilder_initialized` — Creates standalone MCP app via
`create_app()`
The RuntimeError in branch 2 is intentionally defensive. If `appbuilder` is
already initialized (meaning Superset's main app exists), but there's no app
context at the time `flask_singleton.py` is imported, calling `create_app()`
would invoke `appbuilder.init_app()` a second time with a different Flask
instance, overwriting the security manager, views, and other shared state. This
is the exact singleton corruption bug this guard prevents.
In practice, when MCP runs in-process with Superset, the module is imported
during app startup (inside an app context), so branch 1 is taken. When MCP runs
standalone, `appbuilder` is not initialized, so branch 3 is taken. Branch 2
would only trigger if someone imports `flask_singleton.py` after Superset has
fully started but outside any request — which would be a bug in the import
order, and failing loudly is the correct behavior.
--
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]
