Skip to content

Commit 1adcb29

Browse files
feat(kernel): honor _connection_uri and _port on the use_kernel path
The kernel branch of Session._create_backend forwarded only server_hostname and http_path, so _connection_uri and _port were silently ignored on use_kernel=True (connection reached server_hostname/http_path with no error). Add _kernel_host_and_path(): decompose _connection_uri into the kernel host (scheme+authority) + http_path, and fold _port into the host authority. No kernel change needed — the kernel Session host accepts a fully-qualified https://host:port and its normalise_host preserves scheme and port. PECOBLR-4151. Co-authored-by: Isaac Signed-off-by: eric-wang-1990 <e.wang@databricks.com>
1 parent 8f4daee commit 1adcb29

2 files changed

Lines changed: 123 additions & 3 deletions

File tree

src/databricks/sql/session.py

Lines changed: 56 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import logging
22
import re
33
from typing import Dict, Tuple, List, Optional, Any, Type
4+
from urllib.parse import urlsplit
45

56
from databricks.sql.thrift_api.TCLIService import ttypes
67
from databricks.sql.types import SSLOptions
@@ -20,6 +21,56 @@
2021
logger = logging.getLogger(__name__)
2122

2223

24+
def _kernel_host_and_path(
25+
server_hostname: str, http_path: str, kwargs: dict
26+
) -> Tuple[str, str]:
27+
"""Resolve the ``(host, http_path)`` the kernel ``Session`` should use,
28+
honoring the Thrift-style ``_connection_uri`` / ``_port`` overrides.
29+
30+
The kernel derives its endpoint from ``host`` + ``http_path`` and accepts a
31+
fully-qualified ``host`` (its ``normalise_host`` preserves the scheme and
32+
any port), so both overrides can be expressed connector-side without a
33+
kernel change:
34+
35+
- ``_connection_uri`` (a full ``scheme://host[:port]/path`` URI, mirroring
36+
the Thrift backend's direct-URI override) is split into its authority
37+
(returned as ``host``) and its path+query (returned as ``http_path``).
38+
``_connection_uri`` wins over ``_port``, matching the Thrift backend.
39+
- ``_port`` is otherwise folded into the host authority, unless the
40+
hostname already carries a port.
41+
42+
Neither override is set on the common path, so the connection's
43+
``server_hostname`` / ``http_path`` pass through unchanged.
44+
"""
45+
connection_uri = kwargs.get("_connection_uri")
46+
if connection_uri:
47+
# Ensure a scheme so urlsplit populates netloc rather than path; the
48+
# Thrift backend defaults a scheme-less URI to https, so do the same.
49+
uri = connection_uri if "://" in connection_uri else "https://" + connection_uri
50+
parts = urlsplit(uri)
51+
host = "{}://{}".format(parts.scheme, parts.netloc)
52+
path = parts.path or http_path
53+
if parts.query:
54+
path = "{}?{}".format(path, parts.query)
55+
return host, path
56+
57+
port = kwargs.get("_port")
58+
if port is not None:
59+
# Split off any scheme so we can inspect the authority; the kernel
60+
# re-adds https:// when it is absent. Only append the port when the
61+
# authority does not already carry one.
62+
scheme_match = re.match(r"^(https?://)(.*)$", server_hostname)
63+
scheme = scheme_match.group(1) if scheme_match else ""
64+
authority = (scheme_match.group(2) if scheme_match else server_hostname).rstrip(
65+
"/"
66+
)
67+
if ":" not in authority:
68+
authority = "{}:{}".format(authority, port)
69+
return "{}{}".format(scheme, authority), http_path
70+
71+
return server_hostname, http_path
72+
73+
2374
class Session:
2475
def __init__(
2576
self,
@@ -195,9 +246,12 @@ def _create_backend(
195246
"_retry_stop_after_attempts_duration"
196247
),
197248
}
249+
kernel_host, kernel_http_path = _kernel_host_and_path(
250+
server_hostname, http_path, kwargs
251+
)
198252
return KernelDatabricksClient(
199-
server_hostname=server_hostname,
200-
http_path=http_path,
253+
server_hostname=kernel_host,
254+
http_path=kernel_http_path,
201255
http_headers=all_headers,
202256
auth_provider=auth_provider,
203257
ssl_options=self.ssl_options,

tests/unit/test_session.py

Lines changed: 67 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@
99
)
1010
from databricks.sql.backend.types import SessionId, BackendType
1111
from databricks.sql.common.agent import KNOWN_AGENTS
12-
from databricks.sql.session import Session
12+
from databricks.sql.session import Session, _kernel_host_and_path
1313

1414
import databricks.sql
1515

@@ -587,3 +587,69 @@ def test_connect_use_kernel_instantiates_real_kernel_backend(self):
587587
)
588588
finally:
589589
conn.close()
590+
591+
592+
class TestKernelHostAndPathOverrides:
593+
"""``_kernel_host_and_path`` maps the Thrift-style ``_connection_uri`` /
594+
``_port`` overrides onto the kernel ``Session``'s ``host`` + ``http_path``.
595+
596+
Pure-function tests (no kernel wheel / pyarrow needed) covering the
597+
connector-side handling that lets these overrides work on use_kernel=True
598+
without any kernel change.
599+
"""
600+
601+
HOST = "foo.cloud.databricks.com"
602+
PATH = "/sql/1.0/warehouses/abc"
603+
604+
def test_no_override_passes_through(self):
605+
assert _kernel_host_and_path(self.HOST, self.PATH, {}) == (self.HOST, self.PATH)
606+
607+
def test_connection_uri_split_into_authority_and_path(self):
608+
host, path = _kernel_host_and_path(
609+
self.HOST,
610+
self.PATH,
611+
{"_connection_uri": "https://direct.example.com:8443/sql/1.0/warehouses/xyz"},
612+
)
613+
assert host == "https://direct.example.com:8443"
614+
assert path == "/sql/1.0/warehouses/xyz"
615+
616+
def test_connection_uri_without_scheme_defaults_to_https(self):
617+
host, path = _kernel_host_and_path(
618+
self.HOST, self.PATH, {"_connection_uri": "direct.example.com/sql/1.0/warehouses/xyz"}
619+
)
620+
assert host == "https://direct.example.com"
621+
assert path == "/sql/1.0/warehouses/xyz"
622+
623+
def test_connection_uri_preserves_query(self):
624+
host, path = _kernel_host_and_path(
625+
self.HOST,
626+
self.PATH,
627+
{"_connection_uri": "https://h.example.com/sql/1.0/warehouses/xyz?o=123"},
628+
)
629+
assert host == "https://h.example.com"
630+
assert path == "/sql/1.0/warehouses/xyz?o=123"
631+
632+
def test_connection_uri_wins_over_port(self):
633+
host, path = _kernel_host_and_path(
634+
self.HOST,
635+
self.PATH,
636+
{"_connection_uri": "https://direct.example.com:9999/p", "_port": 8443},
637+
)
638+
assert host == "https://direct.example.com:9999"
639+
assert path == "/p"
640+
641+
def test_port_folded_into_bare_host(self):
642+
host, path = _kernel_host_and_path(self.HOST, self.PATH, {"_port": 8443})
643+
assert host == "{}:8443".format(self.HOST)
644+
assert path == self.PATH
645+
646+
def test_port_preserves_existing_scheme(self):
647+
host, path = _kernel_host_and_path(
648+
"https://" + self.HOST, self.PATH, {"_port": 8443}
649+
)
650+
assert host == "https://{}:8443".format(self.HOST)
651+
assert path == self.PATH
652+
653+
def test_port_not_double_appended_when_host_has_port(self):
654+
host, _ = _kernel_host_and_path(self.HOST + ":7000", self.PATH, {"_port": 8443})
655+
assert host == self.HOST + ":7000"

0 commit comments

Comments
 (0)