codeant-ai-for-open-source[bot] commented on code in PR #38229:
URL: https://github.com/apache/superset/pull/38229#discussion_r3440635096
##########
superset/initialization/__init__.py:
##########
@@ -35,6 +35,7 @@
from flask_babel import lazy_gettext as _, refresh
from flask_compress import Compress
from flask_session import Session
+from sqlalchemy.engine import URL
Review Comment:
**Suggestion:** The newly added `URL` import is not used anywhere in this
module, which introduces dead code and can fail strict lint/CI checks. Remove
the unused import to keep the module clean and avoid import-quality gate
failures. [import error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Ruff linting fails due to unused URL import.
- ⚠️ CI or pre-commit pipelines can be blocked.
- ⚠️ Minor code cleanliness regression in initialization module.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open `superset/initialization/__init__.py` and observe the line `from
sqlalchemy.engine
import URL` at line 38 (found via Grep), and verify with `rg "URL\("
superset/initialization/__init__.py` that `URL` is never referenced in this
module.
2. Note that `URL` is legitimately used elsewhere (e.g.
`superset/db_engine_specs/singlestore.py:26`), confirming the import in
`superset/initialization/__init__.py` is not needed for this file's
functionality.
3. Inspect the project's lint configuration in `pyproject.toml` under
`[tool.ruff.lint]`,
which enables `F` codes (Pyflakes) including `F401` for unused imports
(lines 2-27 show
`select = [...] "F" [...]`).
4. Run Ruff against the project (e.g. `ruff check
superset/initialization/__init__.py`
using the configured `pyproject.toml`), and observe an `F401` warning/error
for
`superset/initialization/__init__.py:38` indicating `URL` is imported but
unused, causing
lint checks and any CI/pre-commit pipelines relying on Ruff to fail until
the unused
import is removed.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=4ab07cc6869949388ff1c03e2a7f4e87&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=4ab07cc6869949388ff1c03e2a7f4e87&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:** 38:38
**Comment:**
*Import Error: The newly added `URL` import is not used anywhere in
this module, which introduces dead code and can fail strict lint/CI checks.
Remove the unused import to keep the module clean and avoid import-quality gate
failures.
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%2F38229&comment_hash=f93a17cb5320631b891a709fa905ce6c3b770228b75c21d88c1f28b8e926a22d&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38229&comment_hash=f93a17cb5320631b891a709fa905ce6c3b770228b75c21d88c1f28b8e926a22d&reaction=dislike'>👎</a>
##########
superset/initialization/__init__.py:
##########
@@ -811,7 +812,12 @@ def check_and_warn_database_connection(self) -> None:
db.engine.execute("SELECT 1")
except Exception:
db_uri = self.database_uri
- safe_uri = make_url_safe(db_uri) if db_uri else "Not configured"
+ safe_uri = (
+ make_url_safe(db_uri).render_as_string(hide_password=True)
+ if db_uri
+ else "Not configured"
+ )
Review Comment:
**Suggestion:** The URI-masking path inside the exception handler can raise
`DatabaseInvalidError` when `SQLALCHEMY_DATABASE_URI` is malformed, which will
escape this method and break initialization even though this method is intended
to only warn. Wrap the masking call in a safe fallback (for example,
placeholder text on parse failure) so startup continues after printing the
warning. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ App initialization fails on malformed metadata DB URIs.
- ⚠️ CLI commands abort before printing masked DB warning.
- ⚠️ Error handling diverges from warn-only method intent.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Configure a malformed metadata DB URI in the Superset config (e.g. via
`SQLALCHEMY_DATABASE_URI`), which is read by
`SupersetAppInitializer.database_uri` at
`superset/initialization/__init__.py:58-75` (confirmed via BulkRead; it
returns the raw
string if it doesn't contain fallback markers like `nouser`).
2. Start Superset by calling `create_app()` in `superset/app.py:48-31` (e.g.
any CLI
command or WSGI entrypoint), which constructs `SupersetAppInitializer` and
immediately
invokes `app_initializer.init_app()`.
3. Inside `SupersetAppInitializer.init_app()` at
`superset/initialization/__init__.py:47-66`, the method calls
`self.check_and_warn_database_connection()` (line 65), which enters a `try`
block,
accesses `db.engine.execute("SELECT 1")` (lines 31-33), and on the malformed
URI this call
(or underlying engine creation) raises an exception that is caught by the
`except
Exception:` at line 34.
4. In the `except` block at `superset/initialization/__init__.py:35-40`,
`db_uri =
self.database_uri` retrieves the malformed string and `safe_uri =
(make_url_safe(db_uri).render_as_string(hide_password=True) if db_uri else
"Not
configured")` calls `make_url_safe`; `make_url_safe` at
`superset/databases/utils.py:14-19` wraps SQLAlchemy's `make_url` and raises
`DatabaseInvalidError` on parse failure, which is not caught here, so
`DatabaseInvalidError` propagates out of
`check_and_warn_database_connection()` and
`init_app()` into `create_app()` (which logs and re-raises at
`superset/app.py:46-49`),
turning the intended warning-only check into a hard initialization failure
whenever
`SQLALCHEMY_DATABASE_URI` is malformed.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=67c7db00f3b6429d8bb75b8ad44bb5ae&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=67c7db00f3b6429d8bb75b8ad44bb5ae&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:** 815:819
**Comment:**
*Logic Error: The URI-masking path inside the exception handler can
raise `DatabaseInvalidError` when `SQLALCHEMY_DATABASE_URI` is malformed, which
will escape this method and break initialization even though this method is
intended to only warn. Wrap the masking call in a safe fallback (for example,
placeholder text on parse failure) so startup continues after printing the
warning.
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%2F38229&comment_hash=cbf28c47668c3ace9f96d0f909b6e4b1c5215cae672e6c42e8fc2f617dd852ef&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38229&comment_hash=cbf28c47668c3ace9f96d0f909b6e4b1c5215cae672e6c42e8fc2f617dd852ef&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]