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>
   
   [![Fix in 
Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](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)
 [![Fix in VSCode 
Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](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>
   
   [![Fix in 
Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](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)
 [![Fix in VSCode 
Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](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]

Reply via email to