bito-code-review[bot] commented on code in PR #44710:
URL: https://github.com/apache/superset/pull/44710#discussion_r4120056555


##########
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:
   <!-- Bito Reply -->
   The suggestion is appropriate and improves the code by preventing a 
potential crash. By normalizing non-dictionary `client_info` values to an empty 
dictionary `{}`, the code ensures that `_handle_oauth2` can safely perform 
dictionary lookups, leading to a conservative token purge rather than an 
unhandled exception.
   
   **superset/commands/database/update.py**
   ```
   if not self._model or not client_info or not isinstance(client_info, dict):
               return client_info or {}
   ```



-- 
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