Skip to content

Commit dcb6b0b

Browse files
committed
refactor(kernel): pass max connections directly
Signed-off-by: Vu Anh Phung <vu.phung@databricks.com>
1 parent 5bdda6a commit dcb6b0b

3 files changed

Lines changed: 13 additions & 35 deletions

File tree

CONNECTION_PARAMETERS.md

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ to change without notice.
102102
| ------------------------------------ | ----------- | :----: | :----: | ------------- | ---------------------------------------------------------------------------------------------------------------------------------------------- |
103103
| `_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. |
104104
| `_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. |
105-
| `_pool_maxsize` | `int` ||| `20` (Thrift); `100` (kernel when unset) | Max idle connections retained per host. An explicit value configures the shared Python HTTP client and, when supported by the installed kernel wheel, the kernel's Rust HTTP pool. |
105+
| `_pool_maxsize` | `int` ||| `20` (Thrift); `100` (kernel when unset) | Max idle connections retained per host. An explicit value configures both the shared Python HTTP client and the kernel's Rust HTTP pool. |
106106
| `_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). |
107107
| `_retry_stop_after_attempts_count` | `int` ||| `30` | Max attempts in a retry sequence. Bounded to `[1, 60]` on Thrift; forwarded to the kernel's retry policy. |
108108
| `_retry_stop_after_attempts_duration`| `float` (s) ||| `900` | Max total wall-clock seconds spent retrying. Forwarded to the kernel. |
@@ -204,8 +204,7 @@ None — the kernel's parameter surface is currently a subset of Thrift's.
204204

205205
- **Connection pooling / proxy**: `_pool_connections` and `_proxy_auth_method`
206206
configure only the shared Python HTTP client. `_pool_maxsize` also configures
207-
the kernel's Rust HTTP pool when explicitly set and supported by the installed
208-
kernel wheel.
207+
the kernel's Rust HTTP pool when explicitly set.
209208
- **`use_inline_params`** renders parameters inline on Thrift; the kernel uses
210209
native parameter binding.
211210

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

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ def _kernel_session_accepts_kwarg(name: str) -> bool:
156156
``**kwargs`` catch-all), so forwarding a kwarg it doesn't declare raises
157157
``TypeError`` at construction, so we gate kwargs on what the installed
158158
wheel supports. Falls **closed** (returns ``False``) when the signature
159-
can't be introspected because omitting an accepted optional kwarg is safer
159+
can't be introspected because omitting an accepted telemetry kwarg is safer
160160
than forwarding an unsupported one.
161161
"""
162162
try:
@@ -400,11 +400,6 @@ def open_session(
400400
]
401401
if forwarded:
402402
http_headers_kwargs["http_headers"] = forwarded
403-
pool_kwargs: Dict[str, Any] = {}
404-
if self._max_connections is not None and _kernel_session_accepts_kwarg(
405-
"max_connections"
406-
):
407-
pool_kwargs["max_connections"] = self._max_connections
408403
self._kernel_session = _kernel.Session(
409404
host=self._server_hostname,
410405
http_path=self._http_path,
@@ -423,7 +418,7 @@ def open_session(
423418
# strings).
424419
intervals_as_string=True,
425420
request_timeout_secs=self._request_timeout_secs,
426-
**pool_kwargs,
421+
max_connections=self._max_connections,
427422
**auth_kwargs,
428423
**tls_kwargs,
429424
**retry_kwargs,

tests/unit/test_kernel_client.py

Lines changed: 9 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -368,13 +368,8 @@ def fake_session(**kw):
368368
assert captured["request_timeout_secs"] == timeout
369369

370370

371-
@pytest.mark.parametrize(
372-
("max_connections", "kernel_accepts_kwarg", "expected_forwarded"),
373-
[(None, True, False), (41, True, True), (41, False, False)],
374-
)
375-
def test_open_session_gates_max_connections_for_kernel_compatibility(
376-
monkeypatch, max_connections, kernel_accepts_kwarg, expected_forwarded
377-
):
371+
@pytest.mark.parametrize("max_connections", [None, 41])
372+
def test_open_session_passes_max_connections_to_kernel(monkeypatch, max_connections):
378373
captured = {}
379374

380375
def fake_session(**kw):
@@ -384,11 +379,6 @@ def fake_session(**kw):
384379
return sess
385380

386381
monkeypatch.setattr(kernel_client._kernel, "Session", fake_session)
387-
monkeypatch.setattr(
388-
kernel_client,
389-
"_kernel_session_accepts_kwarg",
390-
lambda name: name == "max_connections" and kernel_accepts_kwarg,
391-
)
392382
c = kernel_client.KernelDatabricksClient(
393383
server_hostname="example.cloud.databricks.com",
394384
http_path="/sql/1.0/warehouses/abc",
@@ -399,10 +389,7 @@ def fake_session(**kw):
399389

400390
c.open_session(session_configuration=None, catalog=None, schema=None)
401391

402-
if expected_forwarded:
403-
assert captured["max_connections"] == max_connections
404-
else:
405-
assert "max_connections" not in captured
392+
assert captured["max_connections"] == max_connections
406393

407394

408395
def test_open_session_passes_phase_7_telemetry_kwargs_to_kernel(monkeypatch):
@@ -471,18 +458,14 @@ def test_open_session_omits_phase_7_kwargs_kernel_does_not_accept(monkeypatch):
471458
them.
472459
473460
The real ``databricks_sql_kernel.Session`` is a PyO3 class with a fixed
474-
signature; the pinned ``^0.2.0`` wheel predates phase 7 and accepts none
475-
of these kwargs, so forwarding them unconditionally raises ``TypeError``
476-
and breaks every ``use_kernel=True`` connection. The other tests here use
477-
a ``**kwargs`` MagicMock that silently swallows the kwargs and hides the
478-
break; this one uses a fixed-signature fake mirroring the real 0.2.0
479-
surface to prove the client gates on what the installed Session supports.
461+
signature. The other tests here use a ``**kwargs`` MagicMock that silently
462+
swallows unsupported kwargs; this fixed-signature fake proves the client
463+
filters optional telemetry kwargs.
480464
"""
481465
captured = {}
482466

483-
# Fixed signature mirroring the pinned 0.2.0 kernel Session: it accepts
484-
# the base connection/tls/retry kwargs but NONE of the phase-7 identity
485-
# or telemetry kwargs, and has no **kwargs catch-all.
467+
# Accept baseline connection/tls/retry/pool kwargs but no phase-7 identity
468+
# or telemetry kwargs, and no **kwargs catch-all.
486469
def fake_session_v0_2_0(
487470
host,
488471
http_path,
@@ -511,6 +494,7 @@ def fake_session_v0_2_0(
511494
complex_types_as_json=False,
512495
intervals_as_string=False,
513496
request_timeout_secs=None,
497+
max_connections=None,
514498
):
515499
captured["host"] = host
516500
sess = MagicMock()

0 commit comments

Comments
 (0)