feat(kernel): forward _pool_maxsize

[vuanhphung]

What type of PR is this?

• Feature

Description

Forward positive _pool_maxsize values to the kernel PyO3 max_connections option. Unset or 0 keeps each HTTP client's default: 20 for the shared Python client and 100 for the kernel.

Kernel support landed in databricks/databricks-sql-kernel#311. The kernel value is honored only with kernel ≥ 1.1.0. Older 1.x wheels remain compatible because the connector omits max_connections when the installed fixed-signature Session does not expose it.

How is this tested?

• Unit tests

177 passed, 1 deselected

Related Tickets & Documents

• databricks/databricks-sql-kernel#311

[peco-review-bot]

Verdict: 1 Medium

Small, well-tested change that threads _pool_maxsize → kernel max_connections, correctly omitting the kwarg when unset (verified: no default is injected at the kwargs layer, so kwargs.get("_pool_maxsize") is None when unset). One medium concern: the kwarg is forwarded to the PyO3 Session without the signature-gating guard the file uses for phase-7 kwargs — worth confirming the ^1.0.0 floor wheel actually accepts max_connections, else an older-but-allowed wheel breaks when the value is set.

[peco-review-bot]

✅ No issues identified by the review bot.

[peco-review-bot]

Verdict: 1 High

Mechanically the change is sound — _pool_maxsize → max_connections threading is correct, unset yields None (kernel default 100, matching the docs), and unit tests cover both the client and session layers. One high concern: max_connections is forwarded unconditionally to the fixed-signature PyO3 Session while the kernel dependency floor is still ^1.0.0, so a resolved 1.0.0 wheel would TypeError on every use_kernel connection unless the floor is bumped or the kwarg is version-gated.

[peco-review-bot]

✅ No issues identified by the review bot.

[peco-review-bot]

Verdict: 1 Low

Looks good — clean, well-tested feature that mirrors the existing request_timeout_secs threading and correctly gates the new max_connections kwarg behind _kernel_session_accepts_kwarg so older 1.x kernel wheels don't break. One low-severity note: the doc now advertises unconditional kernel support while the dependency pin is still ^1.0.0, so configured values are silently dropped on 1.0.x wheels until the floor is bumped (acknowledged as a pre-release step in the PR description).

[peco-review-bot]

Verdict: 1 Low

Looks good — a small, well-tested change that forwards _pool_maxsize to the kernel's max_connections, mirroring the existing request_timeout_secs forwarding and correctly gating on _kernel_session_accepts_kwarg so older fixed-signature wheels stay compatible. One low-severity note about the 0/falsy edge-case divergence between the shared Python client (coerces to 20) and the raw value forwarded to the kernel.

[peco-review-bot]

Verdict: 1 Low

Looks good — clean, well-tested forwarding of _pool_maxsize to the kernel's max_connections, correctly gated on installed-Session support for backward compat and with matching unit tests (None/0/positive). One Low note: the or None guard forwards negative values through unchanged, which the "positive only" contract arguably shouldn't.

[peco-review-bot]

🔵 Low — kwargs.get("_pool_maxsize") or None only normalizes None and 0 to None; a negative value (e.g. _pool_maxsize=-1) is truthy and is forwarded verbatim to the kernel's max_connections. The PR contract is "positive value is forwarded; unset or 0 keeps the default," so a negative should arguably also collapse to the default rather than reaching the kernel's Rust HTTP-pool config (where it may error at Session construction or be interpreted unexpectedly). Consider gating on > 0 instead:

max_connections=(_v if (_v := kwargs.get("_pool_maxsize")) and _v > 0 else None)

or validating upstream. Low severity since negative values are user error on an internal underscore-prefixed param and would fail loud rather than corrupt data.