Skip to content

Commit eb1af56

Browse files
ai: apply changes for #876 (2 review threads)
Addresses: - #3636172662 at src/databricks/sql/auth/oauth.py:173 - #3636172666 at src/databricks/sql/auth/oauth.py:187 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>
1 parent 2d6f729 commit eb1af56

3 files changed

Lines changed: 15 additions & 6 deletions

File tree

src/databricks/sql/auth/authenticators.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import abc
22
import logging
3-
from typing import Callable, Dict, List
3+
from typing import Callable, Dict, List, Optional
44
from databricks.sql.common.http import HttpHeader
55
from databricks.sql.auth.oauth import (
66
OAuthManager,
@@ -63,6 +63,7 @@ def __init__(
6363
scopes: List[str],
6464
http_client,
6565
auth_type: str = "databricks-oauth",
66+
redirect_callback_timeout_seconds: Optional[int] = None,
6667
):
6768
try:
6869
idp_endpoint = get_oauth_endpoints(hostname, auth_type == "azure-oauth")
@@ -74,11 +75,15 @@ def __init__(
7475
# Convert to the corresponding scopes in the corresponding IdP
7576
cloud_scopes = idp_endpoint.get_scopes_mapping(scopes)
7677

78+
# Pass through the (optional) callback-timeout override so this
79+
# private provider can surface it later if a public connection
80+
# knob is added; None falls back to OAuthManager's default.
7781
self.oauth_manager = OAuthManager(
7882
port_range=redirect_port_range,
7983
client_id=client_id,
8084
idp_endpoint=idp_endpoint,
8185
http_client=http_client,
86+
redirect_callback_timeout_seconds=redirect_callback_timeout_seconds,
8287
)
8388
self._hostname = hostname
8489
self._scopes_as_str = DatabricksOAuthProvider.SCOPE_DELIM.join(cloud_scopes)

src/databricks/sql/auth/oauth.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import base64
2+
import errno
23
import hashlib
34
import json
45
import logging
@@ -204,7 +205,11 @@ def _on_timeout():
204205
self.redirect_port = port
205206
break
206207
except OSError as e:
207-
if e.errno == 48:
208+
# errno.EADDRINUSE resolves to the platform's value (48 on
209+
# macOS, 98 on Linux), so a port-in-use bind failure is
210+
# recognized cross-platform. Otherwise last_error stays None on
211+
# Linux and `raise last_error` below becomes `raise None`.
212+
if e.errno == errno.EADDRINUSE:
208213
logger.info(f"Port {port} is in use")
209214
last_error = e
210215
except Exception as e:

tests/unit/test_auth.py

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -350,10 +350,9 @@ def run():
350350
# closing the probe socket and the HTTPServer bind inside
351351
# __get_authorization_code, another process can claim the port. That
352352
# would take the can't-bind branch instead of the stalled-request-
353-
# line path this test exercises (and the source's errno==48 check is
354-
# macOS-only, so on Linux the bind failure surfaces as a TypeError
355-
# rather than the RuntimeError asserted below). Re-probe a fresh port
356-
# and retry on that rare race so the test stays deterministic in CI.
353+
# line path this test exercises, surfacing a bind error rather than
354+
# the RuntimeError asserted below. Re-probe a fresh port and retry
355+
# on that rare race so the test stays deterministic in CI.
357356
worker, result = None, {}
358357
for _ in range(5):
359358
probe = socket.socket(socket.AF_INET, socket.SOCK_STREAM)

0 commit comments

Comments
 (0)