[vuanhphung]

What type of PR is this?

• Other (performance)

Description

The first connect() in a process imports databricks.sql.client, and with it the whole Python HTTP and OAuth client stack: requests, urllib3, oauthlib, jwt and pybreaker. That happens even when nothing uses them. A kernel connection (use_kernel=True) never does, because the kernel owns HTTP, auth and telemetry export. Most of these imports were dead or only needed further down one code path:

• common/http.py imported requests, HTTPAdapter, urllib3's Retry and DatabricksRetryPolicy without using them, and auth/common.py imported DatabricksRetryPolicy without using it. cloudfetch/downloader.py imported UnifiedHttpClient without using it. These are dropped.
• auth/authenticators.py built OAuthManager / ClientCredentialsTokenSource (oauthlib, jwt) in two providers, and auth/auth.py wraps providers in TokenFederationProvider (jwt). These are now imported where they are constructed.
• telemetry_client.py imported UnifiedHttpClient (urllib3) and the push clients (pybreaker), which only TelemetryClient.__init__ uses, so they are imported there. It and the kernel auth bridge also imported TokenFederationProvider for isinstance checks. Those checks now go through sys.modules: an instance can't exist before its module is imported.
• client.py resolves UnifiedHttpClient lazily through the module __getattr__ it already uses for the Thrift names, so patch("databricks.sql.client.UnifiedHttpClient") keeps working. session.py only needed it for an annotation.

Behavior is unchanged: Thrift and SEA connections import what they use when they construct it. After this, a kernel connect and query import none of these modules. Related: #1049 defers pandas the same way.

How is this tested?

• Unit tests
• Manually

New tests import databricks.sql, client, session, auth.auth, auth.authenticators, common.http, the cloudfetch downloader, telemetry_client and the kernel client in fresh interpreters, and assert that none of the five packages loads. They fail on main. More tests check that the token-federation check doesn't import token federation, still recognizes a real TokenFederationProvider (telemetry still reports a wrapped PAT as PAT), and that the browser OAuth provider still builds its OAuthManager. The full unit suite, the realkernel set and mypy pass.

Fresh processes, interleaved, median of 5 (import) or 3 (burst):

	main (0c1b9e8)	this PR
import databricks.sql.client, site-packages with .pyc	360 ms	303 ms
same, site-packages without .pyc (simulated)	1435 ms	1244 ms
	with #1049	with #1049 + this PR
import databricks.sql.client	199 ms	134 ms
200 threads released together from a barrier, connect(use_kernel=True), local mock with 1 ms latency: connect() p50	403 ms	297 ms
same, all sessions open	0.44 s	0.34 s

In a separate isolated live run (4 processes × 200 sessions from a cold start, 16 cores, main vs this PR), each session's open time measured from process launch had a p50 of 1.68 s instead of 1.91 s, peak RSS was 9–11 MB lower per process, and QPS was unchanged.

"Without .pyc" means PYTHONDONTWRITEBYTECODE=1 with a bytecode cache prefix that holds only the stdlib. That is what an install without compiled bytecode looks like, e.g. uv pip install without --compile-bytecode into a location the process can't write.

Live benchmark (staging Large Reyden warehouse)

TPC-H Q6 on a Large Real-Time warehouse, 800 sessions (4 processes x 200 threads), cold start, each session querying back to back for 120 s, 16 client cores. Isolated A/B: only this change vs connector main 0c1b9e8 + kernel 1.4.0, 3 interleaved rounds, nothing else on the warehouse. Measured at 8320850. Median (min-max):

	main	this PR
Average QPS (setup included)	2200 (1968-2226)	2153 (2129-2282)
Steady QPS (after all sessions open)	2238 (1999-2265)	2193 (2162-2321)
Query latency p50 (ms)	348 (348-361)	357 (336-360)
Client CPU per query (ms)	1.32 (1.18-1.36)	1.21 (1.19-1.35)
Session open p50, from process start (s)	1.93 (1.90-1.97)	1.81 (1.79-2.15)
Peak RSS per process (MB)	389 (385-396)	378 (377-387)

QPS within noise; small connect-time and memory gain.

Related Tickets & Documents

RE-17633 (connection-creation time at high concurrency).

This pull request and its description were written by Isaac.

[peco-review-bot]

Verdict: 1 Low

Looks good, with one low concern: the new import-guard test can skip when it should fail. I checked the removed module-level names (common.http, telemetry_client, authenticators, session, downloader). Nothing in src/ or tests/ imports or patches them, and the client.__getattr__ route still supports patch("databricks.sql.client.UnifiedHttpClient").

[peco-review-bot]

Verdict: 1 Low

Looks good: one low-severity concern. Some names that were importable at module level before are now silently gone, outside the client.py __getattr__ shim. The deferred imports, the sys.modules-based isinstance check and the subprocess tests with sentinel exit codes look correct. Nit: in authenticators.py the if TYPE_CHECKING: block sits in the middle of the runtime import list; moving it after the last import would read better.

[peco-review-bot]

✅ No issues identified by the review bot.

[peco-review-bot]

Verdict: 1 Nit

Looks good. I found no correctness issues. The dropped imports in common/http.py, auth/common.py and cloudfetch/downloader.py have no remaining users in the repo. The PEP 562 __getattr__ re-exports cover every moved name, so imports and mock.patch keep working. The sys.modules-based token-federation check is sound, because an instance can't exist until its module has been imported.

Other findings

• ⚪ Nit — Nit: the lazy-name pattern is written three ways: importlib.import_module with a _LAZY_NAMES map in telemetry_client.py, an if name in (...) tuple in auth.py, authenticators.py and downloader.py, and if name == ... in client.py and session.py. One shared helper would make it easier to keep the names in sync. Also, dir() on these modules no longer lists the moved names, which matters only for introspection.

[vuanhphung]

On the lazy-name Nit: in cbdae71, auth, authenticators and the cloudfetch downloader now use the same _LAZY_NAMES map as telemetry_client. client.py and session.py keep the if name == ... style their existing __getattr__ already used for the Thrift and backend names; I only added one branch to each. dir() no longer listing the moved names is left as is, since nothing in the repo introspects them.

[peco-review-bot]

Verdict: 1 Low

Looks good. The lazy-import refactor is correct: the dropped imports in common/http.py and auth/common.py had no remaining users, the PEP 562 __getattr__ keeps every previously exposed name importable and patchable, and the sys.modules check for token federation is sound. One low-severity test-coverage gap. Nit: in authenticators.py, import importlib comes before import abc, and the TYPE_CHECKING block sits in the middle of the import list.

[peco-review-bot]

✅ No issues identified by the review bot.