Skip to content

Commit 7e3e520

Browse files
ai: apply changes for #876 (2 review threads)
Addresses: - #3635960114 at src/databricks/sql/auth/oauth.py:70 - #3635960121 at src/databricks/sql/auth/oauth.py:91 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>
1 parent 3afd559 commit 7e3e520

2 files changed

Lines changed: 19 additions & 11 deletions

File tree

src/databricks/sql/auth/oauth.py

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,12 @@ class OAuthManager:
6767
# would block forever in a headless environment (e.g. a notebook/job with
6868
# no browser), making the connection appear to hang indefinitely. See issue
6969
# #458. This is generous for interactive logins (slow MFA, IdP re-auth, SSO
70-
# redirects); environments that need longer can override it by passing
71-
# ``redirect_callback_timeout_seconds`` to ``__init__``.
70+
# redirects) and is a fixed ceiling for connector end users: there is no
71+
# public ``connect()``/``Connection`` parameter to change it. The
72+
# ``redirect_callback_timeout_seconds`` argument below is an internal-only
73+
# override for callers constructing ``OAuthManager`` directly (private API);
74+
# it is not plumbed through ``DatabricksOAuthProvider`` or the public
75+
# connection kwargs.
7276
REDIRECT_CALLBACK_TIMEOUT_SECONDS = 60 * 5
7377

7478
def __init__(
@@ -84,11 +88,15 @@ def __init__(
8488
self.redirect_port = None
8589
self.idp_endpoint = idp_endpoint
8690
self.http_client = http_client
87-
# Fall back to the class default when not overridden. Set as an instance
88-
# attribute so the rest of the flow reads a single source of truth via
89-
# ``self.REDIRECT_CALLBACK_TIMEOUT_SECONDS``.
90-
if redirect_callback_timeout_seconds is not None:
91-
self.REDIRECT_CALLBACK_TIMEOUT_SECONDS = redirect_callback_timeout_seconds
91+
# Fall back to the class default when not overridden. Kept as a
92+
# lowercase instance attribute (distinct from the ALL_CAPS class
93+
# constant that provides the default) so the rest of the flow reads a
94+
# single, clearly per-instance source of truth.
95+
self._redirect_callback_timeout_seconds = (
96+
redirect_callback_timeout_seconds
97+
if redirect_callback_timeout_seconds is not None
98+
else self.REDIRECT_CALLBACK_TIMEOUT_SECONDS
99+
)
92100

93101
@staticmethod
94102
def __token_urlsafe(nbytes=32):
@@ -152,7 +160,7 @@ def __get_authorization_code(self, client, auth_url, scope, state, challenge):
152160
# Bound how long we wait for the browser redirect callback so
153161
# that a headless environment (no browser to complete the
154162
# flow) fails with a clear error instead of hanging forever.
155-
httpd.timeout = self.REDIRECT_CALLBACK_TIMEOUT_SECONDS
163+
httpd.timeout = self._redirect_callback_timeout_seconds
156164

157165
# HTTPServer.handle_request() returns normally (via
158166
# handle_timeout()) when the wait elapses without a
@@ -196,7 +204,7 @@ def _on_timeout():
196204
if not handler.request_path:
197205
if callback_timed_out:
198206
msg = (
199-
f"Timed out after {self.REDIRECT_CALLBACK_TIMEOUT_SECONDS} "
207+
f"Timed out after {self._redirect_callback_timeout_seconds} "
200208
f"seconds waiting for the OAuth redirect callback at "
201209
f"{redirect_url}. No browser completed the login flow — this "
202210
"is expected in a headless environment (e.g. a notebook or "

tests/unit/test_auth.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -235,7 +235,7 @@ def test_get_tokens_does_not_hang_when_no_callback_received(
235235
)
236236
# Keep the test fast: shorten the callback wait. The production default
237237
# is minutes; the bug is that WITHOUT any bound the wait is infinite.
238-
oauth_manager.REDIRECT_CALLBACK_TIMEOUT_SECONDS = 2
238+
oauth_manager._redirect_callback_timeout_seconds = 2
239239

240240
# No callback is ever delivered to the redirect server. Run the flow in
241241
# a daemon thread and join with a wall-clock bound so a regression to
@@ -271,7 +271,7 @@ def run():
271271
error_message = str(result.get("error"))
272272
self.assertIn("Timed out", error_message)
273273
self.assertIn(
274-
str(oauth_manager.REDIRECT_CALLBACK_TIMEOUT_SECONDS), error_message
274+
str(oauth_manager._redirect_callback_timeout_seconds), error_message
275275
)
276276

277277

0 commit comments

Comments
 (0)