codeant-ai-for-open-source[bot] commented on code in PR #41513:
URL: https://github.com/apache/superset/pull/41513#discussion_r3489635090
##########
superset/db_engine_specs/databend.py:
##########
@@ -404,3 +368,14 @@ def _mutate_label(label: str) -> str:
:return: Conditionally mutated label
"""
return f"{label}_{hash_from_str(label)[:6]}"
+
+
+# Backwards-compatible alias. Previously there were two separate specs
+# (a legacy ``DatabendEngineSpec`` without parameter support and a
+# ``DatabendConnectEngineSpec`` with it), both registered under the
+# ``databend`` engine. Because neither declared distinct ``drivers``,
+# ``get_engine_spec`` resolved to the first-defined (legacy) spec, which
+# lacks ``parameters_schema``/``build_sqlalchemy_uri`` and broke the
+# "configure via individual parameters" flow. They are now merged into a
+# single spec; this alias keeps existing imports working.
+DatabendConnectEngineSpec = DatabendEngineSpec
Review Comment:
**Suggestion:** This module-level class alias is treated as a second engine
spec by `load_engine_specs()` because it iterates `module.__dict__` and
collects every class attribute, so the same Databend spec gets loaded twice.
That creates duplicate processing and can skew diagnostics/metadata tooling
that counts or iterates specs. Keep backward compatibility without exposing a
second class entry (for example by deduplicating in the loader or by providing
a module `__getattr__` alias instead of a class binding in `__dict__`). [code
quality]
<details>
<summary><b>Severity Level:</b> Minor ๐งน</summary>
```mdx
- โ ๏ธ Metadata completeness report double-counts Databend engine spec.
- โ ๏ธ Diagnostic tooling iterates Databend spec twice unnecessarily.
```
</details>
<details>
<summary><b>Steps of Reproduction โ
</b></summary>
```mdx
1. With the PR code deployed, note the alias at
`superset/db_engine_specs/databend.py:381`
where `DatabendConnectEngineSpec = DatabendEngineSpec` binds a second module
attribute
name to the same `DatabendEngineSpec` class defined at lines 180โ241.
2. In `superset/db_engine_specs/__init__.py:51-59`, `is_engine_spec(obj)`
returns True for
any class that is a `BaseEngineSpec` subclass and not `BaseEngineSpec`
itself, without
checking for duplicate identities.
3. In `superset/db_engine_specs/__init__.py:62-76`, `load_engine_specs()`
imports each
engine module and extends `engine_specs` with every attribute in
`module.__dict__` for
which `is_engine_spec(getattr(module, attr))` is True. For the Databend
module, both
attributes `DatabendEngineSpec` and `DatabendConnectEngineSpec` (the alias)
point to the
same class object, so the list comprehension yields that class twice and
`engine_specs`
contains two identical entries.
4. Run the metadata tooling via `get_all_engine_specs()` in
`superset/db_engine_specs/lint_metadata.py:12-24`, which iterates `for spec
in
load_engine_specs():` and appends each spec to `specs` without
deduplication. The
resulting sorted list contains the Databend engine spec twice, and the
report printed in
`print_report()` at `lint_metadata.py:33-40` counts Databend twice in `Total
engine
specs`, demonstrating the duplicate discovery caused by the alias.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=d04ee90e25824d259a0ff71dafda2c72&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=d04ee90e25824d259a0ff71dafda2c72&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/db_engine_specs/databend.py
**Line:** 381:381
**Comment:**
*Code Quality: This module-level class alias is treated as a second
engine spec by `load_engine_specs()` because it iterates `module.__dict__` and
collects every class attribute, so the same Databend spec gets loaded twice.
That creates duplicate processing and can skew diagnostics/metadata tooling
that counts or iterates specs. Keep backward compatibility without exposing a
second class entry (for example by deduplicating in the loader or by providing
a module `__getattr__` alias instead of a class binding in `__dict__`).
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%2F41513&comment_hash=3ebb1f7252b6f2dad06d460f353e62998296b8931cc9eaef4471a9402f246090&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41513&comment_hash=3ebb1f7252b6f2dad06d460f353e62998296b8931cc9eaef4471a9402f246090&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]
