codeant-ai-for-open-source[bot] commented on code in PR #40445:
URL: https://github.com/apache/superset/pull/40445#discussion_r3304875239
##########
superset/initialization/__init__.py:
##########
@@ -846,6 +846,14 @@ def configure_fab(self) -> None:
appbuilder.security_manager_class = custom_sm
appbuilder.init_app(self.superset_app, db.session)
Review Comment:
**Suggestion:** The app-root source order is reversed from the app creation
contract: this reads `APPLICATION_ROOT` first and only falls back to
`SUPERSET_APP_ROOT`. In setups where env var intentionally overrides config,
this computes the wrong prefix for Swagger paths and causes route
mismatches/404s. Resolve using the same precedence as app bootstrap (effective
app root first, then config fallback) so middleware and OpenAPI path generation
stay consistent. [incorrect variable usage]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Prefixed deployments get mismatched OpenAPI/Swagger spec URLs.
- ⚠️ Reverse-proxied Swagger UI may see 404 for spec.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Configure conflicting app roots as in
`tests/unit_tests/initialization_test.py:46-58`:
set `SUPERSET_APP_ROOT=/from-env` in the environment and patch
`superset.config.APPLICATION_ROOT` to `"/from-config"`.
2. Start Superset via `create_app()` in `superset/app.py:48-78`; per lines
63-68,
`app_root` is resolved as `superset_app_root or SUPERSET_APP_ROOT or
APPLICATION_ROOT`, so
the effective app root (and `AppRootMiddleware.app_root`,
`superset/app.py:35-37` and
`192-26`) becomes `"/from-env"`.
3. Observe that in this scenario `app.config["APPLICATION_ROOT"]` remains
`"/from-config"`
because the conditional at `superset/app.py:69-75` only overwrites
`APPLICATION_ROOT` when
it equals `"/"`; with a non-root config value it is not updated to the env
value.
4. During initialization, `SupersetAppInitializer.configure_fab` in
`superset/initialization/__init__.py:833-37` executes the new line 848
`app_root =
self.superset_app.config.get("APPLICATION_ROOT") or
os.environ.get("SUPERSET_APP_ROOT")`,
which resolves to `"/from-config"` and ignores the effective app root
`"/from-env"`, so
any Swagger/OpenAPI prefix computed from this value will not match the active
`AppRootMiddleware` prefix, leading to mismatched paths (for example,
`/from-config/api/v1/_openapi` vs the expected `/from-env/api/v1/_openapi`
used in
`tests/integration_tests/base_api_tests.py:70-74`).
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=cdac2e404c4b49d981f36bfe8f165264&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=cdac2e404c4b49d981f36bfe8f165264&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/initialization/__init__.py
**Line:** 848:848
**Comment:**
*Incorrect Variable Usage: The app-root source order is reversed from
the app creation contract: this reads `APPLICATION_ROOT` first and only falls
back to `SUPERSET_APP_ROOT`. In setups where env var intentionally overrides
config, this computes the wrong prefix for Swagger paths and causes route
mismatches/404s. Resolve using the same precedence as app bootstrap (effective
app root first, then config fallback) so middleware and OpenAPI path generation
stay consistent.
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.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40445&comment_hash=a83daf3061b4b9606a85a568c576eb0a42efb57555b939d70e143f4e51335a2e&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40445&comment_hash=a83daf3061b4b9606a85a568c576eb0a42efb57555b939d70e143f4e51335a2e&reaction=dislike'>👎</a>
##########
superset/initialization/__init__.py:
##########
@@ -846,6 +846,14 @@ def configure_fab(self) -> None:
appbuilder.security_manager_class = custom_sm
appbuilder.init_app(self.superset_app, db.session)
+ # Ensure OpenAPI/Swagger endpoints respect prefix routing (Issue
#33304)
+ app_root = self.superset_app.config.get("APPLICATION_ROOT") or
os.environ.get("SUPERSET_APP_ROOT")
+ if app_root and app_root.strip("/"):
+ prefix = f"/{app_root.strip('/')}"
+ for blueprint in self.superset_app.blueprints.values():
Review Comment:
**Suggestion:** Updating `blueprint.url_prefix` here is too late to fix
routing because blueprint rules are already bound when FAB initializes the app;
changing the attribute after registration does not rebuild URL rules. As a
result, the intended prefixed OpenAPI routes are not actually registered and
the fix can silently fail. Apply the prefix before blueprint registration or
update the URL map/rule generation path directly. [incomplete implementation]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Swagger prefix fix silently fails; routes stay unprefixed.
- ⚠️ Prefixed deployments still hit 404 on OpenAPI endpoints.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. During app initialization, REST APIs are registered on the FAB appbuilder
in
`superset/initialization/__init__.py:270-25` via multiple
`appbuilder.add_api(...)` calls,
and then `appbuilder.init_app(self.superset_app, db.session)` is invoked in
`configure_fab` at `superset/initialization/__init__.py:26-28`, which
registers the
corresponding Flask blueprints (including the `openapi` blueprint) and
populates
`self.superset_app.url_map` with fixed URL rules like `"/api/v1/_openapi"`
(validated by
`tests/integration_tests/base_api_tests.py:67-74`).
2. After FAB has already registered these blueprints,
`SupersetAppInitializer.configure_fab` continues executing and reaches the
new loop at
`superset/initialization/__init__.py:851-853`, iterating `for blueprint in
self.superset_app.blueprints.values():` and detecting the OpenAPI blueprint
by `if
blueprint.name == "openapi":`.
3. Inside this branch, the code sets `blueprint.url_prefix =
f"{prefix}{blueprint.url_prefix or ''}"` (line 853), but does not
re-register the
blueprint or modify `self.superset_app.url_map.rules`; in Flask, the
`url_prefix` is only
consumed when `app.register_blueprint` is called, and mutating the attribute
afterwards
does not retroactively update existing rules.
4. Because routing decisions in Flask are based solely on `app.url_map` and
not on the
current `blueprint.url_prefix` attribute, requests still match the original,
un-prefixed
OpenAPI routes (e.g., `"/api/v1/_openapi"` as used in
`tests/integration_tests/base_api_tests.py:70-74`), so the attempted
prefixing in
`configure_fab` has no effect and the Swagger/OpenAPI endpoints remain
unadjusted for
custom app roots.
```
</details>
[Fix in
Cursor](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=55e82cda95914321ac59a8ec17467570&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
| [Fix in VSCode
Claude](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=55e82cda95914321ac59a8ec17467570&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/initialization/__init__.py
**Line:** 851:853
**Comment:**
*Incomplete Implementation: Updating `blueprint.url_prefix` here is too
late to fix routing because blueprint rules are already bound when FAB
initializes the app; changing the attribute after registration does not rebuild
URL rules. As a result, the intended prefixed OpenAPI routes are not actually
registered and the fix can silently fail. Apply the prefix before blueprint
registration or update the URL map/rule generation path directly.
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.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40445&comment_hash=bd12ddb67d59fbe88a5bef15f0b7f300ea885c977474fa65ec6c312515b335bc&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F40445&comment_hash=bd12ddb67d59fbe88a5bef15f0b7f300ea885c977474fa65ec6c312515b335bc&reaction=dislike'>👎</a>
--
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]
