Skip to content

Commit 7a3ac7c

Browse files
ai: apply changes for #925 (1 review thread)
Addresses: - #3877505593 at src/databricks/sql/backend/kernel/client.py:194 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>
1 parent a406b9e commit 7a3ac7c

2 files changed

Lines changed: 61 additions & 4 deletions

File tree

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

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -184,14 +184,20 @@ def _kernel_session_accepts_kwarg(name: str) -> bool:
184184
(``driver_name`` etc.) only exist on wheels newer than the pinned
185185
``^0.2.0`` (whose ``Session`` accepts none of them), so we must gate them
186186
on what the actually-installed wheel supports rather than pass them
187-
unconditionally. Falls open (returns ``True``) only when the signature
188-
can't be introspected, so a future non-introspectable binding still gets
189-
the kwargs.
187+
unconditionally. Falls **closed** (returns ``False``) when the signature
188+
can't be introspected: a PyO3 class only exposes ``__text_signature__``
189+
(and thus an introspectable signature) when built with
190+
``#[pyo3(signature=...)]``; otherwise ``inspect.signature`` raises
191+
``ValueError``. Since the pinned ``^0.2.0`` ``Session`` accepts none of
192+
these kwargs, forwarding one it doesn't declare is a hard ``TypeError`` at
193+
construction that breaks every ``use_kernel=True`` connection, whereas
194+
omitting one the wheel *would* have accepted only loses telemetry
195+
richness — so we omit the kwarg on introspection failure.
190196
"""
191197
try:
192198
params = inspect.signature(_kernel.Session).parameters
193199
except (TypeError, ValueError):
194-
return True
200+
return False
195201
if any(p.kind is inspect.Parameter.VAR_KEYWORD for p in params.values()):
196202
return True
197203
return name in params

tests/unit/test_kernel_client.py

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -519,6 +519,57 @@ def fake_session_v0_2_0(
519519
assert captured["host"] == "example.cloud.databricks.com"
520520

521521

522+
def test_kernel_session_accepts_kwarg_falls_closed_when_not_introspectable(monkeypatch):
523+
"""When ``inspect.signature(_kernel.Session)`` raises (a PyO3 class built
524+
without ``#[pyo3(signature=...)]`` exposes no ``__text_signature__``, so
525+
``inspect.signature`` raises ``ValueError``), the gate must fall
526+
**closed** and omit every phase-7 kwarg.
527+
528+
Forwarding a kwarg the installed ``Session`` doesn't declare is a hard
529+
``TypeError`` at construction that breaks every ``use_kernel=True``
530+
connection; omitting one it would have accepted only loses telemetry
531+
richness. The fixed-signature fake in the sibling test always introspects,
532+
so this test uses a stand-in whose signature genuinely can't be read to
533+
cover the real non-introspectable PyO3 binding.
534+
"""
535+
536+
class NonIntrospectableSession:
537+
# Mirrors a PyO3 class with no exposed __text_signature__:
538+
# inspect.signature() raises ValueError on it.
539+
def __init__(self, *args, **kwargs): # pragma: no cover - never called
540+
pass
541+
542+
def raise_value_error(_obj):
543+
raise ValueError("no signature found for builtin type")
544+
545+
monkeypatch.setattr(kernel_client._kernel, "Session", NonIntrospectableSession)
546+
monkeypatch.setattr(kernel_client.inspect, "signature", raise_value_error)
547+
548+
assert kernel_client._kernel_session_accepts_kwarg("driver_name") is False
549+
550+
monkeypatch.setattr(
551+
kernel_client.TelemetryHelper,
552+
"get_driver_system_configuration",
553+
lambda: types.SimpleNamespace(
554+
driver_name="Databricks SQL Python Connector",
555+
driver_version="1.2.3",
556+
runtime_name="Python 3.12.0",
557+
runtime_version="3.12.0",
558+
runtime_vendor="CPython",
559+
os_name="Linux",
560+
os_version="6.1",
561+
os_arch="x86_64",
562+
client_app_name=None,
563+
locale_name="en_US",
564+
char_set_encoding="utf-8",
565+
),
566+
)
567+
kwargs = kernel_client._kernel_telemetry_kwargs(
568+
{"enable_telemetry": True, "telemetry_batch_size": 17}
569+
)
570+
assert kwargs == {}, f"expected no phase-7 kwargs when signature unreadable, got {kwargs}"
571+
572+
522573
def test_execute_command_forwards_parameters_to_bind_param():
523574
"""``execute_command(parameters=[...])`` routes each parameter
524575
through ``bind_tspark_params`` onto the kernel statement before

0 commit comments

Comments
 (0)