GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Cache kernel Session compatibility signature by aakash-saravanan-db · Pull Request #1024 · databricks/databricks-sql-python

Conversation

@aakash-saravanan-db

@aakash-saravanan-db aakash-saravanan-db commented Nov 11, 2026

Copy link
Copy Markdown
Collaborator

What type of PR is this?

  • Other: Performance

Description

Cache the native Session constructor's supported parameters across connections and threads instead of inspecting its signature for every optional-argument check.

Initialize the cache under a lock and refresh it when the Session class changes. Cache introspection failures as unsupported, and preserve **kwargs handling.

How is this tested?

  • Unit tests

Added coverage for signature reuse, concurrent initialization, class replacement, **kwargs, and cached TypeError/ValueError results.

Local validation: the full unit suite, separate real-kernel loopback tests, mypy, and Black.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@aakash-saravanan-db
aakash-saravanan-db had a problem deploying to azure-prod November 11, 2026 19:43 — with GitHub Actions Error
@aakash-saravanan-db
Cache kernel Session compatibility signature
94698e7
Share constructor parameter support across threads and cache introspection failures. Refresh the cached support when the native Session class changes.

Signed-off-by: Aakash Saravanan <[email protected]>
@aakash-saravanan-db
aakash-saravanan-db force-pushed the aakash-saravanan/cache-kernel-session-signature branch from f927645 to 94698e7 Compare November 11, 2026 19:56
@aakash-saravanan-db
aakash-saravanan-db temporarily deployed to azure-prod November 11, 2026 19:56 — with GitHub Actions Inactive
@aakash-saravanan-db
aakash-saravanan-db temporarily deployed to azure-prod November 11, 2026 19:56 — with GitHub Actions Inactive
@aakash-saravanan-db
aakash-saravanan-db marked this pull request as ready for review November 11, 2026 22:04
@aakash-saravanan-db
aakash-saravanan-db temporarily deployed to azure-prod November 11, 2026 22:04 — with GitHub Actions Inactive
peco-review-bot[bot]
peco-review-bot Bot approved these changes Nov 11, 2026
View reviewed changes

@peco-review-bot peco-review-bot Bot left a comment

Copy link
Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verdict: 1 Low

Looks good, with one low-severity note about test isolation. The cache is checked once without the lock and again under it, it is keyed on the identity of the Session object, and it keeps the existing behaviour: unreadable signatures still count as unsupported, and **kwargs is still honoured. The new tests cover reuse, concurrent first use, class replacement and the TypeError/ValueError paths.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Comment thread tests/unit/test_kernel_client.py


def test_kernel_session_accepts_kwarg_falls_closed_when_not_introspectable(monkeypatch):
def test_kernel_session_signature_is_reused(monkeypatch):

@peco-review-bot peco-review-bot Bot Nov 11, 2026

Copy link
Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Low — The new process-global cache _KERNEL_SESSION_PARAMETER_SUPPORT is never reset between tests. The current tests only work because each one installs a brand-new Session class or function, so the identity check forces a refresh. If a later test patches only inspect.signature, or reuses a shared Session such as the module-level MagicMock, it will silently read a result cached by an earlier test. That test then depends on run order and may not exercise the path it claims to test. The signature.assert_called_once_with(...) assertions would also break under that reuse. Add an autouse fixture that does monkeypatch.setattr(kernel_client, "_KERNEL_SESSION_PARAMETER_SUPPORT", None), at least for the signature-gate tests, so each test starts with an empty cache.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@peco-review-bot
peco-review-bot Bot temporarily deployed to azure-prod November 11, 2026 22:05 Inactive

This branch was previously deployed

1 inactive deployment
azure-prod — 94698e71 Deployed Nov 11, 2026 by peco-review-bot[bot] via followup #1528
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@peco-review-bot peco-review-bot[bot] peco-review-bot[bot] approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

1 participant

@aakash-saravanan-db
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.