Skip to content

Commit 17c7789

Browse files
authored
feat(kernel): forward socket timeout (#931)
* feat(kernel): forward socket timeout Signed-off-by: Vu Anh Phung <vu.phung@databricks.com> * fix(kernel): validate request timeout Signed-off-by: Vu Anh Phung <vu.phung@databricks.com> * refactor(kernel): rely on timeout binding validation Signed-off-by: Vu Anh Phung <vu.phung@databricks.com> * refactor(kernel): pass request timeout directly Signed-off-by: Vu Anh Phung <vu.phung@databricks.com> --------- Signed-off-by: Vu Anh Phung <vu.phung@databricks.com>
1 parent c77f275 commit 17c7789

7 files changed

Lines changed: 40 additions & 9 deletions

File tree

CONNECTION_PARAMETERS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ to change without notice.
9999
100100
| Option | Type | Thrift | Kernel | Default Value | Note |
101101
| ------------------------------------ | ----------- | :----: | :----: | ------------- | ---------------------------------------------------------------------------------------------------------------------------------------------- |
102-
| `_socket_timeout` | `float` (s) || | `900` | Socket send/recv/connect timeout. Not forwarded to the kernel, which manages its own request timeout. |
102+
| `_socket_timeout` | `float` (s) || | `900` (Thrift); `120` (kernel) | Thrift: socket send/recv/connect timeout. Kernel: total HTTP request deadline from connect through response-body completion. A positive value is forwarded; unset or `0` selects the kernel's 120s default. On the kernel path, `0` is neither unlimited nor an immediate timeout. |
103103
| `_pool_connections` | `int` || ⚠️ | `10` | Number of urllib3 connection pools. Configures the connector's shared Python HTTP client; the kernel's query transport is its own Rust stack. |
104104
| `_pool_maxsize` | `int` || ⚠️ | `20` | Max connections per pool on the shared Python HTTP client. Same kernel caveat as `_pool_connections`. |
105105
| `_proxy_auth_method` | `str` || ⚠️ | `None` | `basic` or `negotiate` (Kerberos). Applies to the shared Python HTTP client; not threaded to the kernel query transport. See [`docs/proxy.md`](docs/proxy.md). |

KERNEL_REV

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
ad78a5be3dc8bb7fc78ec574492515ab24e23d4c
1+
dd810d6d0a179886b923c6e22dc785ddca16ebef

src/databricks/sql/backend/kernel/client.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,8 @@ def __init__(
217217
# to the kernel ``Session``'s ``retry_*`` kwargs in
218218
# ``open_session`` via ``_kernel_retry_kwargs``.
219219
self._retry_options = kwargs.get("retry_options") or {}
220+
# The kernel binding owns type and range validation.
221+
self._request_timeout_secs = kwargs.get("request_timeout_secs")
220222
self._catalog = catalog
221223
self._schema = schema
222224
# ``_use_arrow_native_complex_types`` is the connector-side
@@ -369,6 +371,7 @@ def open_session(
369371
# backend's surface (interval columns arrive as
370372
# strings).
371373
intervals_as_string=True,
374+
request_timeout_secs=self._request_timeout_secs,
372375
**auth_kwargs,
373376
**tls_kwargs,
374377
**retry_kwargs,

src/databricks/sql/client.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -273,8 +273,10 @@ def read(self) -> Optional[OAuthToken]:
273273
# _retry_stop_after_attempts_count
274274
# The maximum number of attempts during a request retry sequence (defaults to 24)
275275
# _socket_timeout
276-
# The timeout in seconds for socket send, recv and connect operations. Defaults to None for
277-
# no timeout. Should be a positive float or integer.
276+
# On Thrift, the timeout in seconds for socket send, recv and connect
277+
# operations. On the kernel path, a positive value is the total HTTP
278+
# request deadline. Kernel values of None or 0 select its 120-second
279+
# default; 0 is neither unlimited nor an immediate timeout.
278280
# _disable_pandas
279281
# In case the deserialisation through pandas causes any issues, it can be disabled with
280282
# this flag.

src/databricks/sql/session.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -230,6 +230,7 @@ def _create_backend(
230230
_use_arrow_native_complex_types=_use_arrow_native_complex_types,
231231
auth_options=kernel_auth_options,
232232
retry_options=kernel_retry_options,
233+
request_timeout_secs=kwargs.get("_socket_timeout"),
233234
)
234235

235236
databricks_client_class: Type[DatabricksClient]

tests/unit/test_kernel_client.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -344,6 +344,30 @@ def fake_session(**kw):
344344
assert captured.get("complex_types_as_json") is expected_flag
345345

346346

347+
@pytest.mark.parametrize("timeout", [None, 0, 12.5])
348+
def test_open_session_passes_request_timeout_to_kernel(monkeypatch, timeout):
349+
captured = {}
350+
351+
def fake_session(**kw):
352+
captured.update(kw)
353+
sess = MagicMock()
354+
sess.session_id = "sess-id"
355+
return sess
356+
357+
monkeypatch.setattr(kernel_client._kernel, "Session", fake_session)
358+
c = kernel_client.KernelDatabricksClient(
359+
server_hostname="example.cloud.databricks.com",
360+
http_path="/sql/1.0/warehouses/abc",
361+
auth_provider=AccessTokenAuthProvider("dapi-test"),
362+
ssl_options=None,
363+
request_timeout_secs=timeout,
364+
)
365+
366+
c.open_session(session_configuration=None, catalog=None, schema=None)
367+
368+
assert captured["request_timeout_secs"] == timeout
369+
370+
347371
def test_execute_command_forwards_parameters_to_bind_param():
348372
"""``execute_command(parameters=[...])`` routes each parameter
349373
through ``bind_tspark_params`` onto the kernel statement before

tests/unit/test_session.py

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -410,10 +410,9 @@ def test_use_kernel_pat_builds_minimal_access_token_provider(self):
410410
assert isinstance(sess.auth_provider, AccessTokenAuthProvider)
411411

412412

413-
class TestKernelRetryOptionsThreading:
414-
"""The connector's ``_retry_*`` kwargs must be forwarded into the
415-
kernel client's ``retry_options`` on the use_kernel path (the kernel
416-
owns the retry loop). Captures the kwargs session.py passes by
413+
class TestKernelTransportOptionsThreading:
414+
"""The connector's retry and socket timeout kwargs must be forwarded
415+
on the use_kernel path. Captures the kwargs session.py passes by
417416
patching ``KernelDatabricksClient`` and inspecting its call args.
418417
419418
Patching ``KernelDatabricksClient`` requires importing
@@ -426,7 +425,7 @@ class TestKernelRetryOptionsThreading:
426425

427426
PACKAGE = "databricks.sql"
428427

429-
def test_retry_kwargs_threaded_into_kernel_client(self):
428+
def test_retry_and_socket_timeout_threaded_into_kernel_client(self):
430429
import sys
431430
import types
432431

@@ -466,6 +465,7 @@ def test_retry_kwargs_threaded_into_kernel_client(self):
466465
_retry_delay_max=90.0,
467466
_retry_stop_after_attempts_count=10,
468467
_retry_stop_after_attempts_duration=600.0,
468+
_socket_timeout=12.5,
469469
)
470470
try:
471471
_, kwargs = mock_kernel_client.call_args
@@ -474,6 +474,7 @@ def test_retry_kwargs_threaded_into_kernel_client(self):
474474
assert opts["retry_delay_max"] == 90.0
475475
assert opts["retry_stop_after_attempts_count"] == 10
476476
assert opts["retry_stop_after_attempts_duration"] == 600.0
477+
assert kwargs["request_timeout_secs"] == 12.5
477478
finally:
478479
conn.close()
479480

0 commit comments

Comments
 (0)