codeant-ai-for-open-source[bot] commented on code in PR #37548:
URL: https://github.com/apache/superset/pull/37548#discussion_r2740928831
##########
superset/mcp_service/auth.py:
##########
@@ -32,7 +32,7 @@
from typing import Any, Callable, TYPE_CHECKING, TypeVar
from flask import g
-from flask_appbuilder.security.sqla.models import User
+from flask_appbuilder.security.sqla.models import Group, User
Review Comment:
**Suggestion:** Top-level import of ORM model classes (`Group`, `User`) at
module import time can trigger import-time side effects (like requiring a Flask
app context or creating circular imports). Move runtime imports into functions
or keep them under TYPE_CHECKING to avoid "working outside of application
context" errors and circular import problems. [possible bug]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ MCP module import fails during standalone startup.
- ⚠️ FastMCP tool discovery may error on import.
- ⚠️ Some unit tests importing auth will fail.
```
</details>
```suggestion
# Import model classes at runtime inside functions to avoid import-time side
effects
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open a Python REPL or a process that does not create a Flask app context.
2. Execute: import superset.mcp_service.auth (file:
superset/mcp_service/auth.py line 35).
- This triggers the module top-level import at line 35: "from
flask_appbuilder.security.sqla.models import Group, User".
3. If flask_appbuilder.security.sqla.models requires an application context
or touches
SQLAlchemy
engine setup during import, the import will raise a RuntimeError (e.g.,
"Working
outside
of application context") or other import-time failure.
4. Observed symptom: module import fails before any MCP decorator or runtime
logic runs.
- Reproduces concretely by running `python -c "import
superset.mcp_service.auth"` in
an environment where no Flask app is configured.
Note: The PR's file actually contains this top-level import at line 35, so
the suggested
mitigation (defer import) is relevant if this module is imported in contexts
without a
Flask app.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/mcp_service/auth.py
**Line:** 35:35
**Comment:**
*Possible Bug: Top-level import of ORM model classes (`Group`, `User`)
at module import time can trigger import-time side effects (like requiring a
Flask app context or creating circular imports). Move runtime imports into
functions or keep them under TYPE_CHECKING to avoid "working outside of
application context" errors and circular import problems.
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]
