bito-code-review[bot] commented on code in PR #44710:
URL: https://github.com/apache/superset/pull/44710#discussion_r4113999764
##########
superset/commands/database/update.py:
##########
@@ -219,6 +221,29 @@ def _handle_oauth2(self) -> None:
self._model.purge_oauth2_tokens()
break
+ def _resolve_oauth2_client_info(self, client_info: Any) -> Any:
+ """
+ Fill in the values the engine spec derives, as ``get_oauth2_config``
does.
+
+ The current config has the derived values (e.g. Databricks endpoints
from
+ the workspace host), so the new one needs them too, otherwise an
omitted
+ value would count as a change and purge the tokens on every update. The
+ new connection URI is used, since a new host means new endpoints.
+ """
+ if not self._model or not client_info or not isinstance(client_info,
dict):
+ return client_info
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Non-dict client_info crash</b></div>
<div id="fix">
The new guard returns a non-dict `oauth2_client_info` (e.g. a string inside
user-submitted `encrypted_extra`) unchanged, and `_handle_oauth2` then calls
`new_config.get(key)` at line 220, raising AttributeError (500 on update). The
removed line shows the same crash predates this change, but the new guard can
fix it: normalize non-dicts to `{}` so the comparison conservatively purges
tokens instead.
</div>
</div>
<small><i>Code Review Run #4466c1</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]
