bito-code-review[bot] commented on code in PR #36850:
URL: https://github.com/apache/superset/pull/36850#discussion_r2649912865
##########
superset/db_engine_specs/databricks.py:
##########
@@ -670,6 +870,24 @@ def adjust_engine_params(
return uri, connect_args
+ @classmethod
+ def get_oauth2_authorization_uri(cls, database: "Database") -> str:
+ """
+ Return the OAuth2 authorization URI for this Databricks workspace.
+ Dynamically generated from the database host.
+ """
+ host = cls.get_parameters_from_uri(database.sqlalchemy_uri)["host"]
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>OAuth2 URI Host Handling Bug</b></div>
<div id="fix">
The OAuth2 authorization URI method assumes host is always present and not
None, but get_parameters_from_uri can return None for host, resulting in
invalid URLs like 'https://None/oidc/v1/authorize'. It looks like this should
include a fallback to extract host directly from the URI, similar to the
existing implementation in DatabricksODBCEngineSpec.
</div>
</div>
<small><i>Code Review Run #cb1a2b</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/db_engine_specs/databricks.py:
##########
@@ -670,6 +870,24 @@ def adjust_engine_params(
return uri, connect_args
+ @classmethod
+ def get_oauth2_authorization_uri(cls, database: "Database") -> str:
+ """
+ Return the OAuth2 authorization URI for this Databricks workspace.
+ Dynamically generated from the database host.
+ """
+ host = cls.get_parameters_from_uri(database.sqlalchemy_uri)["host"]
+ return f"https://{host}/oidc/v1/authorize";
+
+ @classmethod
+ def get_oauth2_token_uri(cls, database: "Database") -> str:
+ """
+ Return the OAuth2 token URI for this Databricks workspace.
+ Dynamically generated from the database host.
+ """
+ host = cls.get_parameters_from_uri(database.sqlalchemy_uri)["host"]
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>OAuth2 URI Host Handling Bug</b></div>
<div id="fix">
Similarly, the OAuth2 token URI method has the same host handling issue,
potentially generating invalid URLs if host is None. It appears this should use
the same fallback logic as the authorization URI.
</div>
</div>
<small><i>Code Review Run #cb1a2b</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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]
