codeant-ai-for-open-source[bot] commented on code in PR #44203:
URL: https://github.com/apache/superset/pull/44203#discussion_r3996798310
##########
superset/db_engine_specs/databricks.py:
##########
@@ -311,13 +398,23 @@ class DatabricksODBCEngineSpec(DatabricksBaseEngineSpec):
class DatabricksDynamicBaseEngineSpec(BasicParametersMixin,
DatabricksBaseEngineSpec):
default_driver = ""
encryption_parameters = {"ssl": "1"}
- required_parameters = {"access_token", "host", "port"}
+ # ``access_token`` is required unless Encrypted Extra has M2M credentials.
+ required_parameters = {"host", "port"}
context_key_mapping = {
"access_token": "password",
"host": "hostname",
"port": "port",
}
+ # Mask the service-principal secret; leave ``auth_method`` / ``client_id``
+ # visible when the database is edited (Snowflake / Redshift pattern).
+ # ``$.oauth2_client_info.secret`` is always added by the base class.
+ # pylint: disable=invalid-name
+ encrypted_extra_sensitive_fields = {
+ "$.client_secret": "OAuth Client Secret",
+ "$.azure_client_secret": "Azure Client Secret",
+ }
Review Comment:
**Suggestion:** Replacing the inherited wildcard removes masking for
existing encrypted-extra values, exposing unrelated passwords, tokens, and
secret-valued driver options in database responses. [security]
**Assessment:** ๐ด `Critical` ยท ๐ `Occurrence: Often`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=51a3ea04e091404e8081e1c01860f7c7&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=51a3ea04e091404e8081e1c01860f7c7&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/db_engine_specs/databricks.py
**Line:** 413:416
**Comment:**
*Security: Replacing the inherited wildcard removes masking for
existing encrypted-extra values, exposing unrelated passwords, tokens, and
secret-valued driver options in database responses.
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%2F44203&comment_hash=850e857b62171973a86eaddd9613ebc15fbb5b84d77643625f68fe26df8e604b&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44203&comment_hash=850e857b62171973a86eaddd9613ebc15fbb5b84d77643625f68fe26df8e604b&reaction=dislike'>๐</a>
##########
superset/db_engine_specs/databricks.py:
##########
@@ -478,7 +578,47 @@ def update_params_from_encrypted_extra(
logger.error(ex, exc_info=True)
raise
encrypted_extra.pop("oauth2_client_info", None)
- params.update(encrypted_extra)
+
+ auth_method = encrypted_extra.pop("auth_method", None)
+ client_id = encrypted_extra.pop("client_id", None) or
encrypted_extra.pop(
+ "azure_client_id", None
+ )
+ client_secret = encrypted_extra.pop("client_secret", None) or (
+ encrypted_extra.pop("azure_client_secret", None)
+ )
+ # Always drop the Azure-named aliases so they never reach
create_engine.
+ encrypted_extra.pop("azure_client_id", None)
+ encrypted_extra.pop("azure_client_secret", None)
+ azure_tenant_id = encrypted_extra.pop("azure_tenant_id", None)
+
+ if auth_method == AUTH_METHOD_OAUTH_M2M:
+ if not client_id or not client_secret:
+ raise ValueError(
+ "Databricks OAuth M2M requires both client_id and "
+ "client_secret in Secure Extra."
+ )
+ host = ""
+ if getattr(database, "url_object", None) is not None:
+ host = database.url_object.host or ""
+ connect_args = params.setdefault("connect_args", {})
+ connect_args["credentials_provider"] = (
+ _build_oauth_m2m_credentials_provider(host, client_id,
client_secret)
+ )
Review Comment:
**Suggestion:** A new credentials-provider closure is created for every
engine build, and its memory-based representation changes the engine cache key,
defeating engine reuse for M2M connections. [performance]
**Assessment:** ๐ `Major` ยท ๐ `Occurrence: Often`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=58ca4515f92f4faf9713f8a0bd42a724&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=58ca4515f92f4faf9713f8a0bd42a724&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/db_engine_specs/databricks.py
**Line:** 604:606
**Comment:**
*Performance: A new credentials-provider closure is created for every
engine build, and its memory-based representation changes the engine cache key,
defeating engine reuse for M2M connections.
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%2F44203&comment_hash=77e7634bfa2bc2f8eb5bac58643feda3af79a23bb9eed5086fef7e290825abff&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44203&comment_hash=77e7634bfa2bc2f8eb5bac58643feda3af79a23bb9eed5086fef7e290825abff&reaction=dislike'>๐</a>
##########
superset/db_engine_specs/databricks.py:
##########
@@ -539,10 +696,11 @@ def validate_parameters( # type: ignore
],
) -> list[SupersetError]:
errors: list[SupersetError] = []
- connect_args: dict[str, Any] = {}
- if extra := json.loads(properties.get("extra")): # type: ignore
- engine_params = extra.get("engine_params", {})
- connect_args = engine_params.get("connect_args", {})
+ extra_raw = properties.get("extra")
+ extra = json.loads(extra_raw) if extra_raw else {} # type: ignore
Review Comment:
**Suggestion:** Malformed `extra` JSON raises `JSONDecodeError` from
parameter validation instead of returning validation errors, causing the
database validation endpoint to fail with a server error. [error handling]
**Assessment:** ๐ `Major` ยท ๐ `Occurrence: Sometimes`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=0fb321e55851465d9ad2f4a9879ad887&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=0fb321e55851465d9ad2f4a9879ad887&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/db_engine_specs/databricks.py
**Line:** 700:700
**Comment:**
*Error Handling: Malformed `extra` JSON raises `JSONDecodeError` from
parameter validation instead of returning validation errors, causing the
database validation endpoint to fail with a server error.
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%2F44203&comment_hash=77bc0e720ca25e536ba16cf29930a3960766ecdac26045f608b23d85c4837667&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44203&comment_hash=77bc0e720ca25e536ba16cf29930a3960766ecdac26045f608b23d85c4837667&reaction=dislike'>๐</a>
##########
superset/db_engine_specs/databricks.py:
##########
@@ -478,7 +578,47 @@ def update_params_from_encrypted_extra(
logger.error(ex, exc_info=True)
raise
encrypted_extra.pop("oauth2_client_info", None)
- params.update(encrypted_extra)
+
+ auth_method = encrypted_extra.pop("auth_method", None)
+ client_id = encrypted_extra.pop("client_id", None) or
encrypted_extra.pop(
+ "azure_client_id", None
+ )
+ client_secret = encrypted_extra.pop("client_secret", None) or (
+ encrypted_extra.pop("azure_client_secret", None)
+ )
+ # Always drop the Azure-named aliases so they never reach
create_engine.
+ encrypted_extra.pop("azure_client_id", None)
+ encrypted_extra.pop("azure_client_secret", None)
+ azure_tenant_id = encrypted_extra.pop("azure_tenant_id", None)
+
+ if auth_method == AUTH_METHOD_OAUTH_M2M:
+ if not client_id or not client_secret:
+ raise ValueError(
+ "Databricks OAuth M2M requires both client_id and "
+ "client_secret in Secure Extra."
+ )
+ host = ""
+ if getattr(database, "url_object", None) is not None:
+ host = database.url_object.host or ""
+ connect_args = params.setdefault("connect_args", {})
+ connect_args["credentials_provider"] = (
+ _build_oauth_m2m_credentials_provider(host, client_id,
client_secret)
+ )
+ elif auth_method == AUTH_METHOD_AZURE_SP_M2M:
+ if not client_id or not client_secret:
+ raise ValueError(
+ "Databricks Azure service-principal M2M requires both "
+ "client_id and client_secret in Secure Extra."
+ )
+ connect_args = params.setdefault("connect_args", {})
+ connect_args["auth_type"] = AUTH_METHOD_AZURE_SP_M2M
Review Comment:
**Suggestion:** `azure-sp-m2m` is not a supported Databricks SQL connector
authentication type, so every documented Azure service-principal connection
fails during connector initialization. [api mismatch]
**Assessment:** ๐ด `Critical` ยท ๐ `Occurrence: Sometimes`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=05cb2c1843f8406198903951c683b1f5&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=05cb2c1843f8406198903951c683b1f5&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/db_engine_specs/databricks.py
**Line:** 614:614
**Comment:**
*Api Mismatch: `azure-sp-m2m` is not a supported Databricks SQL
connector authentication type, so every documented Azure service-principal
connection fails during connector initialization.
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%2F44203&comment_hash=790e3b91949396c31c9be769f4e36f092d040821f864ef7af463a8866db2c245&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F44203&comment_hash=790e3b91949396c31c9be769f4e36f092d040821f864ef7af463a8866db2c245&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]
