This repository was archived by the owner on Mar 31, 2026. It is now read-only.
Test - #1
Open
evb123 wants to merge 320 commits into
Open
Conversation
Closes #123 Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
* Add OAuth M2M example Signed-off-by: Jacky Hu <jacky.hu@databricks.com>
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
Signed-off-by: Martin Rueckl <enigma@nbubu.de>
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com> (cherry picked from commit 8d85fa8)
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
pyarrow is currently compatible with Python 3.8 → Python 3.11 I also removed specifiers for when Python is 3.7 since this no longer applies. Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
Rename `dbapi` classmethod to `import_dbapi` as required by SQLAlchemy 2 Closes #289 Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
… a TIMESTAMP_NTZ column (#296) Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
--------- Signed-off-by: Pieter Noordhuis <pieter.noordhuis@databricks.com> Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com> Co-authored-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
Signed-off-by: Jessica <12jessicasmith34@gmail.com> Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com> Co-authored-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
Signed-off-by: Christophe Bornet <cbornet@hotmail.com> Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com> Co-authored-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
…ks (#330) Signed-off-by: Ben Cassell <ben.cassell@databricks.com> Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com> Co-authored-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
--------- Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
Signed-off-by: Jacky Hu <jacky.hu@databricks.com> Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com> Co-authored-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
…341) * Redact the URL query parameters from the urllib3.connectionpool logs Signed-off-by: Mubashir Kazia <mubashir.kazia@databricks.com> * Fix code formatting Signed-off-by: Mubashir Kazia <mubashir.kazia@databricks.com> * Add str check for the log record message arg dict values Signed-off-by: Mubashir Kazia <mubashir.kazia@databricks.com> --------- Signed-off-by: Mubashir Kazia <mubashir.kazia@databricks.com>
Signed-off-by: Jacky Hu <jacky.hu@databricks.com>
* [PECO-1411] Support OAuth InHouse on GCP Signed-off-by: Jacky Hu <jacky.hu@databricks.com> * Update changelog Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com> --------- Signed-off-by: Jacky Hu <jacky.hu@databricks.com> Signed-off-by: Jesse Whitehouse <jesse.whitehouse@databricks.com> Co-authored-by: Jesse Whitehouse <jesse.whitehouse@databricks.com>
* [PECO-1414] Support Databricks InHouse OAuth in Azure Signed-off-by: Jacky Hu <jacky.hu@databricks.com>
Getting ready for test automation Signed-off-by: Ben Cassell <ben.cassell@databricks.com>
* build(kernel): add optional [kernel] extra for use_kernel=True
databricks-sql-kernel is now published to PyPI, so the kernel backend
can ship as an optional dependency instead of a local-dev-only build.
- pyproject: declare databricks-sql-kernel as an optional dependency
gated to python>=3.10 (the wheel is cp310-abi3, Requires-Python
>=3.10), and add the `[kernel]` extra. The extra also lists pyarrow:
the kernel result path (backend/kernel/result_set.py) imports it
unconditionally to wrap the Arrow batches the kernel returns. pyarrow
is already pulled transitively via the kernel wheel's
pyarrow>=23.0.1,<24, but naming it makes the connector-side
requirement explicit and lets pip co-resolve both constraints at
install time.
- backend/kernel/_errors.py: update the use_kernel=True ImportError to
point at `pip install "databricks-sql-connector[kernel]"` and note the
python>=3.10 requirement (was the obsolete "not yet published, build
locally" hint).
- README: document the [kernel] extra, use_kernel=True usage, and the
python>=3.10 / pyarrow notes.
On python<3.10 the `[kernel]` extra resolves to nothing and
use_kernel=True raises the friendly ImportError at runtime; the
connector's own python floor (3.8) is unchanged.
Verified locally (kernel served from a locally-built cp310-abi3 wheel,
since the published package isn't yet mirrored on the dev proxy):
- pip install "databricks-sql-connector[kernel]" -> connector + kernel
+ pyarrow all install; use_kernel=True runs a live query end-to-end
(backend KernelDatabricksClient).
- plain install -> use_kernel=True raises the friendly ImportError.
NOTE: `poetry lock` still needs to be run to refresh poetry.lock with
the databricks-sql-kernel entry; it is intentionally NOT included here
because it requires the kernel to be resolvable on the index poetry/CI
use (the JFrog db-pypi proxy). Confirm the package resolves there before
merging.
Co-authored-by: Isaac
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
* fix(kernel): drop pyarrow from the [kernel] extra to unbreak poetry lock
Listing bare `pyarrow` in the [kernel] extra forced poetry to co-resolve
an unconstrained pyarrow against the kernel's transitive
`pyarrow>=23.0.1,<24` across the connector's full 3.8–3.14 matrix.
pyarrow 23.x requires Python >=3.10, so the constraint is unsatisfiable
on 3.8/3.9 — `poetry lock` failed every CI job with "version solving
failed ... pyarrow is forbidden".
The kernel wheel already declares `pyarrow>=23.0.1,<24` as a hard
runtime dependency, so `pip install databricks-sql-connector[kernel]`
still pulls a compatible pyarrow transitively. The databricks-sql-kernel
dep stays gated to python>=3.10, which now correctly excludes the whole
kernel+pyarrow subtree from the 3.8/3.9 resolution. The kernel's own
metadata is the single source of truth for the pyarrow floor.
Co-authored-by: Isaac
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
* fix(kernel): cap pyarrow <23 on the sub-3.10 band so poetry lock resolves
The kernel's transitive pyarrow>=23.0.1,<24 conflicts with the
connector's own pyarrow>=14.0.1 (declared across 3.8–3.13) during
`poetry lock`: pyarrow>=23 dropped Python 3.9, so for the 3.8–3.10
slice poetry can't find a pyarrow satisfying both and version solving
fails ("pyarrow is forbidden" -> "databricks-sql-kernel is forbidden").
The kernel's python>=3.10 marker doesn't help because poetry unifies
the pyarrow constraint across the connector's declared pyarrow band,
not the kernel's.
Split the connector's pyarrow entry at 3.10 and cap the <3.10 band at
<23. This removes no installable version — the newest pyarrow with a
Python 3.9 wheel is 21.x — it just makes that physical fact explicit to
the solver, so the <3.10 band (capped, kernel absent) and the >=3.10
band (where the kernel can pull pyarrow up to <24) no longer overlap.
Verified `poetry lock` resolves the full dependency set with this
change.
Co-authored-by: Isaac
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
* ci(kernel): add "Unit Tests + Kernel" matrix exercising the real wheel
Mirrors the "Unit Tests + PyArrow" matrix but for the [kernel] extra.
Until now no CI job exercised the published kernel wheel: the base
unit-test matrix installs no extras, and the kernel unit tests use a
fake databricks_sql_kernel module injected into sys.modules, so the
real wheel was never loaded in CI.
The new job (Python 3.10–3.14; the wheel is cp310-abi3 so 3.9 is
omitted) installs the [kernel] extra via --all-extras, then:
- asserts databricks_sql_kernel imports and has a real __file__ (i.e.
the published wheel actually installed, not the test fake), and
- imports the use_kernel backend path (KernelDatabricksClient /
KernelResultSet) against the real wheel,
before running the unit suite. This is the only CI signal that the
published [kernel] extra installs and loads end to end on every PR
(the live use_kernel=True e2e remains in kernel-e2e.yml, merge-queue
gated).
Co-authored-by: Isaac
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
* test(kernel): exercise use_kernel=True through the real wheel; no silent skips
Ensure every CI job that's meant to cover the kernel actually drives
the use_kernel=True path through the REAL databricks-sql-kernel wheel,
and fails loudly if it can't (rather than silently skipping / passing
on the Thrift path).
Problem this fixes:
- The kernel unit tests inject a fake databricks_sql_kernel into
sys.modules. In a shared `pytest tests/unit tests/e2e` session (the
coverage job, which installs --all-extras so the real wheel IS
present) that fake shadowed the real wheel, so the kernel e2e tests
silently skipped — the coverage job looked like it exercised the
kernel but didn't.
Changes:
- tests/e2e/test_kernel_backend.py + test_kernel_tls.py: replace the
silent `__file__`-based skip with a three-state guard keyed on
importlib.metadata (the on-disk dist DB, which a sys.modules stub
can't fake): skip only when the wheel is genuinely absent; FAIL
LOUDLY when it's installed-but-shadowed. The `conn` fixture now also
asserts conn.session.backend is KernelDatabricksClient, so a
use_kernel=True connection that fell back to Thrift fails the test.
- tests/unit/test_session.py: add TestUseKernelRoutesThroughRealWheel
(marked `realkernel`) — a no-network proof that
sql.connect(use_kernel=True) instantiates the REAL
KernelDatabricksClient (mocks only open_session; does not fake the
wheel). Skips if the wheel is absent; fails if it's shadowed.
- pyproject.toml: register the `realkernel` marker. Tests so marked
need an unpolluted sys.modules and must run in a separate pytest
invocation from the fake-injecting unit tests.
- tests/unit/test_kernel_client.py: document that its session-global
fake mandates the separate-invocation rule for real-wheel tests.
- code-quality-checks.yml: the Unit Tests + Kernel matrix now asserts
the real wheel, runs `tests/unit -m "not realkernel"`, then runs the
real-wheel routing test as its own invocation
(`pytest tests/unit/test_session.py -m realkernel`). All three unit
matrices gained `-m "not realkernel"`.
- code-coverage.yml: --ignore the kernel e2e files and add
`-m "not realkernel"` so the shared --all-extras session doesn't trip
the new loud guards; the real live kernel e2e stays in kernel-e2e.yml
(isolated session, real wheel, live warehouse).
Co-authored-by: Isaac
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
* ci(kernel): install per-tier extras explicitly, not --all-extras
The "Unit Tests + PyArrow" job used --all-extras, which predates the
[kernel] extra. Now that [kernel] exists, --all-extras silently also
installs the kernel wheel — so that tier no longer isolated the
"pyarrow present, kernel absent" configuration and overlapped the new
"Unit Tests + Kernel" job.
- Unit Tests + PyArrow: --extras pyarrow (pyarrow only; no kernel).
- Unit Tests + Kernel: --extras kernel (resolves the published
databricks-sql-kernel wheel via the [kernel] extra — the exact edge
`pip install databricks-sql-connector[kernel]` uses — which
transitively brings pyarrow).
Each tier now targets its configuration precisely. The kernel install
path here (published wheel via the extra) is intentionally distinct
from kernel-e2e.yml, which maturin-builds tip-of-tree at KERNEL_REV.
Verified against the proxy: --extras pyarrow installs pyarrow and NOT
the kernel; --extras kernel installs databricks-sql-kernel 0.1.2.
Co-authored-by: Isaac
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
* build(kernel): require databricks-sql-kernel >=0.2.0
Bump the [kernel] extra's floor from ^0.1.0 to ^0.2.0 (>=0.2.0,<0.3.0)
now that 0.2.0 is published. The <0.3.0 cap is deliberate: the kernel is
pre-1.0, so each 0.x minor may be breaking — we bump this when the
kernel ships 0.3.0 rather than auto-adopting a potentially-breaking
minor.
0.2.0 keeps the same Requires-Python (>=3.10) and pyarrow (>=23.0.1,<24)
pin as 0.1.x, so the python>=3.10 marker and the pyarrow <23 sub-3.10
cap are unchanged. Verified `poetry lock` resolves and locks
databricks-sql-kernel 0.2.0.
Co-authored-by: Isaac
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
---------
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
…843) * ci: add DBR LTS install check + fix pyarrow-compat it caught (ES-1960554) The thrift 0.23.0 bump (PR #796, shipped in 4.2.7) broke `pip install` on DBR LTS: thrift ships sdist-only and 0.23.0's setup.py calls sys.exit(0) on the build-success path, killing the PEP 517 backend before pip writes output.json. On the old setuptools shipped by DBR 14.3/15.4 LTS this is a hard install failure (SEV0 ES-1960554); 4.2.7 was yanked and reverted (#840). Our CI never caught it because every job installs via `poetry install` on a modern runner -- it never does a fresh `pip install` of the built wheel on an LTS toolchain, the real customer path that failed. CI check -------- Adds a PR check (gated to dependency changes) that builds the wheel and installs it INSIDE real DBR LTS clusters via the PECO workspace Jobs API (no PyPI publish) then runs a SELECT 1 smoke test. Matrix = supported LTS {13.3, 14.3, 15.4, 16.4, 17.3} x install target {base, pyarrow, kernel}. Auth is OAuth M2M as the PECO service principal throughout (driver -> workspace API and the notebook's connector -> warehouse smoke query); a PAT is warehouse-scoped and rejected by the workspace REST API. Older LTS ship an SDK too old for auth_type=oauth-m2m, so the smoke harness upgrades databricks-sdk. Per-run artifacts are cleaned up in a finally block. Connector fix (caught by the check) ----------------------------------- The check surfaced a real latent bug: a base install (no [pyarrow] extra) runs against a runtime's bundled pyarrow, and on DBR 13.3/14.3 that pyarrow predates the `promote_options` kwarg, so concat_table_chunks raised `TypeError: concat_tables() got an unexpected keyword argument 'promote_options'` on the Arrow result path. utils.py now falls back to the legacy `promote=True` (equivalent to promote_options="default") when the kwarg is unsupported, with a regression test. Validated end-to-end against the PECO workspace: green on thrift 0.22.0, and re-widening the pin to <0.24.0 fails on 14.3+15.4 with the exact output.json error -- a true guard, not a check that always passes. Also adds an incident-linked comment on the thrift pin so nobody re-widens it before the upstream fix (THRIFT-6067 / apache/thrift#3584) ships. Co-authored-by: Isaac Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com> * test(e2e): authenticate e2e via OAuth M2M so staging tests match DATABRICKS_USER The e2e suite connected via a PAT (DATABRICKS_TOKEN). The Personal Staging Location tests PUT/GET/REMOVE against stage://tmp/<DATABRICKS_USER>/..., where DATABRICKS_USER is the PECO service principal (TEST_PECO_SP_ID). A personal stage is identity-scoped by design (there is even a test asserting you cannot touch another user's stage), so the connecting identity MUST equal DATABRICKS_USER. When DATABRICKS_TOKEN authenticates as a different identity, those tests fail with `PERMISSION_DENIED: <user> does not have access to Personal Stage`. Switch the e2e connection to OAuth M2M as the service principal via credentials_provider (conftest.auth_connect_kwargs), so the connecting identity IS the SP == DATABRICKS_USER. Falls back to the PAT when SP OAuth creds aren't set, so local PAT runs are unaffected. Wires DATABRICKS_CLIENT_ID / DATABRICKS_CLIENT_SECRET (TEST_PECO_SP_ID / TEST_PECO_SP_OAUTH_SECRET, already in azure-prod) into code-coverage.yml. Verified locally against the PECO workspace: all 9 staging_ingestion e2e tests pass via the real M2M path (including fails_to_modify_another_staging_user, which validates the identity scoping). Kernel e2e files are unchanged (they run in kernel-e2e.yml, ignored by code-coverage.yml). Co-authored-by: Isaac Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com> * test(e2e): add databricks-sdk dev dependency for OAuth M2M auth The e2e M2M auth (conftest.auth_connect_kwargs) imports databricks.sdk.core.oauth_service_principal, but databricks-sdk was not a project dependency, so `poetry install` in code-coverage.yml didn't provide it -- every e2e connection failed with `ModuleNotFoundError: No module named 'databricks.sdk'`. Add it to the dev group (test-only; not a runtime dep of the connector). CI's setup-poetry runs `poetry lock` before install, so the lockfile is regenerated on the runner. Co-authored-by: Isaac Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com> * Revert e2e OAuth M2M; use SP PAT instead (keep DBR LTS check on M2M) The azure-prod DATABRICKS_TOKEN is now a personal access token owned by the PECO service principal, so its identity matches DATABRICKS_USER. That fixes the Personal Staging Location tests (stage://tmp/<SP>/...) with a plain PAT, without the OAuth M2M machinery -- which also broke the retry/HTTP tests, since M2M makes a live token-endpoint call that those tests' urllib3 mocking intercepts. Reverts the e2e auth changes (conftest.auth_connect_kwargs + the consumer call sites + the databricks-sdk dev dep + the code-coverage.yml SP env) back to the plain access_token path. The DBR LTS install check keeps OAuth M2M: it hits the workspace Jobs/SCIM API (which rejects a warehouse-scoped PAT), is proven 15/15 green on M2M, and installs databricks-sdk itself in its own workflow. Co-authored-by: Isaac Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com> --------- Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
…831) (#845) Fork pull_request runs never receive a GitHub OIDC token, so .github/actions/setup-jfrog (which mints a JFrog PyPI token from that OIDC token) dies with "ACTIONS_ID_TOKEN_REQUEST_TOKEN: unbound variable" and every required check goes red on every fork PR — regardless of the diff (#831, reported against the trivial fork PR #671). Public PyPI is unreachable from the protected runners, so a fork run can obtain no package source at all. Adopt the Databricks SOP (mirrors databricks/dbt-databricks): a trusted warmer workflow that DOES get OIDC pre-builds the in-project .venv for every matrix leg and saves it to the Actions cache; fork PRs restore that .venv and run the tools directly from .venv/bin, fully offline, never touching JFrog. Everything still flows through JFrog (the warmer populates the cache from it); the fork just holds no credentials, so there is no pull_request_target / secret-exposure risk. - .github/workflows/warm-deps-cache.yml: new, sole writer of the forkvenv-* cache. Triggers on push:main (dep files), a daily schedule (beats GitHub's 7-day cache eviction), and workflow_dispatch with an optional pr_number that warms a fork's changed lockfiles (fetches ONLY poetry.lock/pyproject.toml, no source). Warms the full py x depset x extras matrix, priming mypy stubs too. - .github/actions/restore-deps: new composite action; restores the newest forkvenv-* entry for the leg via a restore-keys prefix and reports cache-hit. - .github/workflows/code-quality-checks.yml: each job tries restore-deps first and runs offline from .venv/bin on a hit; on a miss (all same-repo PRs, or a fork whose lockfiles changed pre-warm) it falls back to the original setup-poetry JFrog flow, unchanged. Same-repo PR behavior is unaffected. Scope: unit + lint + type checks (the fork-reachable required checks). e2e / kernel-e2e / integration / DBR-LTS stay merge-queue-gated (they need live warehouse secrets). Closes #831 Co-authored-by: Isaac Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
…l_path (#726) (#846) Fix: REMOVE staging op no longer requires staging_allowed_local_path (#726) A REMOVE staging operation deletes a remote resource and never touches the local filesystem, yet _handle_staging_operation raised a ProgrammingError demanding staging_allowed_local_path before dispatching to any handler. This forced callers to pass a dummy path (e.g. "/") just to run a REMOVE, even though the value is never used. Handle REMOVE before the staging_allowed_local_path validation block, mirroring how the __input_stream__ PUT path already bypasses it. GET and local-file PUT continue to require staging_allowed_local_path. Closes #726 Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
The install-args expression used `matrix.extras == '' && '' || format(...)`. GitHub Actions treats the empty-string middle operand as falsy, so the base leg (extras='') fell through to the format() branch and produced `--extras ` with no value, failing `poetry install` with 'The "--extras" option requires a value'. Only the base legs failed; pyarrow/kernel legs (non-empty extras) passed. Invert to test for non-empty extras first and default to '', which never yields a valueless --extras. Co-authored-by: Isaac Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
…831) (#850) The warmer computed its cache key with hashFiles('**/poetry.lock') AFTER setup-poetry ran `poetry lock`, which rewrites poetry.lock in place. The PR jobs' restore-deps hashes the PRISTINE committed poetry.lock (it runs before setup-poetry). The two hashes therefore never matched — a fork PR looked up e.g. forkvenv-...-46e4852b... while the warmer had saved forkvenv-...-143cf261... so every fork restore missed and fell back to the JFrog path (which forks can't auth to), failing all jobs. Capture the pristine lock hash in a step BEFORE setup-poetry and use that for the cache key, matching what restore-deps computes. Found via the fork verification PR #849 (all Unit Tests legs missed the cache). Refs #831 Co-authored-by: Isaac Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
…#831) (#851) Follow-ups from the fork verification run (#849) once the cache finally hit. The offline restore mechanism works (10 legs green offline incl. all kernel legs); these fix the remaining offline-path failures: 1. Python 3.9 can't be provisioned offline — the protected-runner image preinstalls 3.10-3.14 but not 3.9 (EOL), so setup-python tries to DOWNLOAD it and fails with no network on a fork. That hard-failed the 3.9 legs and, under fail-fast, cancelled the working 3.10-3.14 siblings. Move setup-python into restore-deps with continue-on-error, probe the restored interpreter, and export venv-usable; callers gate their offline run on it and skip 3.9 neutrally (3.9 fork coverage is in the merge queue) instead of failing. Keeps 3.9 in the matrix per maintainer preference without letting it sink the other legs. 2. check-types offline ran , which mypy rejects without --install-types ('--non-interactive is only supported with --install-types'). The warmer already primed the stubs into the venv, so run plain . 3. The warmer excluded 3.14-min but the PR matrix runs 3.14-min pyarrow (PR excludes only 3.12-min/3.13-min) — that leg missed the cache and fell back to JFrog. Align the warmer's excludes to the PR matrix exactly. Refs #831 Co-authored-by: Isaac Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
The fork verification (#849) showed 3 checks still red on fork PRs — pre-existing (also red on the original #671), separate from the cache fix, all rooted in GitHub's fork restrictions: - skip-integration-tests-pr / skip-kernel-e2e-pr post synthetic-success check-runs, needing checks:write. On fork PRs GitHub forces GITHUB_TOKEN to read-only regardless of the declared permission, so checks.create 403s ('Resource not accessible by integration'). Swallow the 403 ONLY for forks (github.event.pull_request.head.repo.fork) so the poster doesn't show a spurious failure; the REAL required checks are posted by the merge_group run (base context, full perms) when a maintainer queues the PR — the fork's code is genuinely integration-tested there. Non-fork/other errors still fail loudly. - test-with-coverage runs the e2e suite against a live warehouse and needs the azure-prod secrets, which forks never get (Secret source: None) — so it could only fail on a fork. Skip it on fork PRs; the real coverage gate runs on the merge_group transient branch (secrets present), which is what protects main. Same-repo PRs and merge_group are unaffected by all three changes. Refs #831 Co-authored-by: Isaac Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
* Support cursor.rowcount for DML on the Thrift backend (#784) cursor.rowcount was hardcoded to -1 and never updated for the Thrift backend (the default). For DML (INSERT/UPDATE/DELETE/MERGE) the Databricks Thrift server reports the affected-row count in TGetOperationStatusResp.numModifiedRows, but the connector discarded it — _wait_until_command_done kept only operationState. Thread numModifiedRows through: _wait_until_command_done now returns the terminal status response, _handle_execute_response reads numModifiedRows from it, ExecuteResponse and ResultSet carry a num_modified_rows field, and Cursor.execute sets self.rowcount from it. rowcount resets to -1 before each statement so a DML count never leaks into a later SELECT. SELECT (and statements the server does not report a count for) leave rowcount at its -1 default. This brings the Thrift path in line with the kernel backend, which already surfaces num_modified_rows. Closes #784 Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com> * test(e2e): verify DML cursor.rowcount against a live warehouse (#784) End-to-end test in TestPySQLCoreSuite exercising INSERT/UPDATE/DELETE on a real Thrift warehouse and asserting cursor.rowcount reports the exact affected-row count (3/2/1), then that a following SELECT resets it to -1. Verified live on dogfood; fails with the fix reverted (rowcount stays -1). Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com> * Aggregate executemany rowcount across parameter sets (#784) Address PR review: executemany looped execute() per parameter set and, since each execute() resets rowcount, left rowcount equal to only the final set's affected-row count. Per PEP 249, rowcount after executemany should reflect the total across all operations. Accumulate the reported per-statement counts; if no statement reports a count (all SELECT / the server reports none), rowcount stays at its -1 default. Adds unit tests for the sum, mixed reported/unreported, and all-unreported cases, and extends the live e2e test to assert executemany aggregation. Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com> --------- Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
* feat(bots): local install-bot-engine + bot-prelude composites (PAT-free) External consumers can't `uses:` the internal engine's actions cross-repo, so install the engine via a LOCAL composite that pip-installs it over HTTPS with a short-lived, engine-scoped GitHub App token (no stored PAT), routed through the internal JFrog mirror. bot-prelude wraps the shared per-workflow setup (mint bot token + engine-scoped token + Node + install) and centralizes the engine pin in one place — engine-ref default, pinned to databricks-bot-engine@3569dae (post Type-flow #107, REPO_ROOT #115, DCO sign-off #117). Signed-off-by: Eric Wang <e.wang@databricks.com> Co-authored-by: Isaac Signed-off-by: eric-wang-1990 <e.wang@databricks.com> * feat(bots): reviewer bot via pinned databricks-bot-engine (PAT-free) Reviewer + reviewer-followup as own-job workflows that install the engine via the local bot-prelude composite (PAT-free App-token auth) and run reviewer_bot.run_review / .followup. Triggers on every non-fork PR; fork gate in each job `if:`. .bot/prompts/review/system.md is repo-specific ADDITIVE guidance appended to the engine's base reviewer prompt. Signed-off-by: Eric Wang <e.wang@databricks.com> Co-authored-by: Isaac Signed-off-by: eric-wang-1990 <e.wang@databricks.com> * feat(bots): engineer bot (bug-fix flow, Type-selected) + onboarding spec Engineer author (label an issue) + followup (review-comment replies) as own-job workflows: setup-poetry for the connector's test deps, engine install via the local bot-prelude composite, then engineer_bot.run. Author flow selected from the issue Type (Bug ⇒ bug-fix; other/none ⇒ .bot/config.yaml default, also bug-fix). REPO_ROOT points the agent's working tree + .bot/ lookup at the checkout. .bot/config.yaml (bug-fix, poetry pytest allowlist, flow-aware templates omitted to use engine defaults) + engineer/engineer-followup prompts. Verified live — reviewer posts findings; engineer author + followup run end to end. docs/superpowers/specs/: onboarding design + first-consumer findings trail. Signed-off-by: Eric Wang <e.wang@databricks.com> Co-authored-by: Isaac Signed-off-by: eric-wang-1990 <e.wang@databricks.com> --------- Signed-off-by: Eric Wang <e.wang@databricks.com> Signed-off-by: eric-wang-1990 <e.wang@databricks.com>
…lish (#866) * fix(bots): revert poetry lock/pyproject churn before author/followup publish The engineer-bot author run on issue #860 failed at "Open / update fix PR": publish's leftover safety-net found poetry.lock + pyproject.toml unstaged and not in the agent's touched_files, so it refused to open the PR — even though the agent's fix + tests were correct (494 passed). Root cause: the shared setup-poetry action runs `poetry lock` (to reconcile the lock with the internal JFrog source it injects), which REWRITES the tracked poetry.lock / pyproject.toml in the working tree. That churn is harmless for the 5 real-CI consumers of the action (they commit nothing) but poisons the two engineer-bot workflows, which publish a PR from the working tree. Fix (engineer-bot side only — leave the shared action untouched): after the Poetry setup, `git checkout -- poetry.lock pyproject.toml` to drop the churn once the venv is built, so only the agent's edits remain in the tree at publish time. Applied to both engineer-bot.yml (author) and engineer-bot-followup.yml. Signed-off-by: eric-wang-1990 <e.wang@databricks.com> Co-authored-by: Isaac * ai: apply changes for #866 (1 review thread) Addresses: - #3599651153 at .github/workflows/engineer-bot.yml:89 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com> --------- Signed-off-by: eric-wang-1990 <e.wang@databricks.com> Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com> Co-authored-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>
…869) A maintainer with PRIVATE org membership is reported as `author_association: CONTRIBUTOR` in the pull_request_review_comment webhook payload (even though REST shows MEMBER). The followup gate only accepted OWNER/MEMBER/COLLABORATOR, so such a maintainer's review comments SILENTLY failed the gate — the job skipped with no error and the bot never engaged. Observed on #868: a review comment (MEMBER per REST) triggered a followup run that skipped, because the payload association was CONTRIBUTOR (membership private). Add CONTRIBUTOR. Low risk: this path already requires a non-fork, OPEN, `engineer-bot`-labeled PR (a maintainer-applied opt-in). Mirrors the engine fix (databricks/databricks-bot-engine#120). Co-authored-by: Isaac Signed-off-by: eric-wang-1990 <e.wang@databricks.com>
…only) (#870) * feat(engineer-bot): require a live E2E repro for bug fixes (not unit-only) The bug-fix flow's red→green discipline doesn't guarantee the bug is *reproduced* — only that the agent's test agrees with the agent's fix. On #868 (retry max→min) the agent wrote/edited MOCKED unit tests to match its own wrong fix; they passed green, but the change violated the real retry contract — caught only by pre-existing, human-authored e2e tests. The engineer prompt here explicitly told the agent "treat the unit suite as your only executable verification" — the opposite of the sibling adbc-drivers/databricks bot, which REQUIRES a live E2E repro. Port that discipline (adapted to Python/pytest/this connector): Prompt (.bot/prompts/engineer/system.md): - An E2E test (tests/e2e/, live warehouse) that reproduces the bug red and verifies the fix green is REQUIRED; a mocked unit test alone is NOT sufficient. blocked (not a unit-test substitute) if the behavior genuinely isn't e2e-observable. - Test-first, reproduction is a HARD GATE (blocked if it can't fail-for-the-right- reason after a focused effort). - Do NOT rewrite an existing test's expectations to agree with the fix (the #868 failure mode); add a new failing test, and justify any existing-assertion change. - Ground expected behavior in an external authority (issue/spec, or the JDBC reference driver via context-repo) — not in the current connector code. - Use a minimal, self-contained, -k-filtered e2e test (the bot job doesn't seed the full fixture set). Workflows (engineer-bot.yml author + engineer-bot-followup.yml run steps): - Pass the 4 live-warehouse connection env vars the e2e suite needs (DATABRICKS_SERVER_HOSTNAME / HTTP_PATH / CATALOG / USER), mirroring code-coverage.yml. The jobs already run in `environment: azure-prod`, so the secrets are in scope — they just weren't mapped into the run step. Signed-off-by: eric-wang-1990 <e.wang@databricks.com> Co-authored-by: Isaac * ai: apply changes for #870 (2 review threads) Addresses: - #3600243617 at .github/workflows/engineer-bot.yml:194 - #3600243618 at .bot/prompts/engineer/system.md:48 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com> * ai: apply changes for #870 (1 review thread) Addresses: - #3600282324 at .bot/prompts/engineer/system.md:33 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com> * ai: apply changes for #870 (2 review threads) Addresses: - #3600313097 at .github/workflows/engineer-bot-followup.yml:155 - #3600313099 at .github/workflows/engineer-bot.yml:88 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com> * ai: apply changes for #870 (1 review thread) Addresses: - #3600339404 at .github/workflows/engineer-bot-followup.yml:107 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com> * ai: apply changes for #870 (1 review thread) Addresses: - #3600361719 at .bot/prompts/engineer/system.md:98 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com> * ai: apply changes for #870 (1 review thread) Addresses: - #3600385831 at .github/workflows/engineer-bot-followup.yml:158 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com> * ai: apply changes for #870 (1 review thread) Addresses: - #3600405525 at .github/workflows/engineer-bot-followup.yml:104 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com> * docs(bots): teach backend selection (Thrift/SEA/kernel) + realkernel tiers Follow-up to the #870 review thread on --all-extras: the bot needs to know, per issue, WHICH backend the bug is on and reproduce on that one — a Thrift bug won't reproduce on a kernel connection, and a broad unit run with the real kernel wheel present false-reds unless realkernel is deselected. That knowledge was tribal; write it down. - CONTRIBUTING.md: add a "Backends and test tiers" section — the three backends (Thrift default / SEA `use_sea=True` / kernel `use_kernel=True`), where each backend's tests live, that kernel is an opt-in extra, and the rule that `realkernel` tests run in their own invocation (`-m "not realkernel"` for broad runs), matching how CI (code-coverage.yml / code-quality-checks.yml) splits them. - engineer/system.md: add step 0 — pick the backend the bug is on and reproduce there; point to the CONTRIBUTING matrix. - engineer-followup/system.md: correct the stale "do NOT run tests/e2e" line (the followup job now has live creds via #870) and point at the same backend matrix. Keeps --all-extras (both backends supported); the residual "prompt-discipline only" risk the reviewer flagged is now backed by a documented, human-shared convention plus explicit bot rules. Signed-off-by: eric-wang-1990 <e.wang@databricks.com> Co-authored-by: Isaac * ai: apply changes for #870 (2 review threads) Addresses: - #3600996714 at .github/workflows/engineer-bot.yml:200 - #3601002346 at CONTRIBUTING.md:156 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com> * ai: apply changes for #870 (2 review threads) Addresses: - #3601040104 at .bot/prompts/engineer-followup/system.md:31 - #3601040111 at .bot/prompts/engineer/system.md:121 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com> --------- Signed-off-by: eric-wang-1990 <e.wang@databricks.com> Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com> Co-authored-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>
…lt fix) (#871) Moves the engine pin (bot-prelude's single engine-ref default) from 3569dae3 to 5368de77, which includes databricks/databricks-bot-engine#122 — the fix for the can_use_tool permission callback returning a dict instead of a PermissionResult object. Without it, every edit_file/bash call the engineer bot makes fails with `TypeError: Tool permission callback must return PermissionResult ... got dict`, so the bot can read but not edit or run tests (observed on #791: two runs hung to the 45-min job timeout). The fix is runtime-verified against the pinned SDK (0.2.102). Co-authored-by: Isaac Signed-off-by: eric-wang-1990 <e.wang@databricks.com>
…t strip (#872) The bug-fix flow's REQUIRED tests/e2e repro authenticates through conftest.py, which reads the token from DATABRICKS_TOKEN. But the engineer-bot runs pytest in an agent-driven subprocess whose environment is scrubbed of every credential- shaped variable (the engine's shared/env_scrub.py strips *TOKEN*/*SECRET*/... ), so DATABRICKS_TOKEN is gone before pytest starts. The connection is then built with access_token=None and hangs — observed on issue #791: every e2e test (incl. existing, unmodified ones) stalled ~10 min per attempt until the 45-min job timeout. The same warehouse/env is used by code-coverage.yml's e2e job, which passes because it runs pytest directly (no agent, no scrub). Fix (per the engine's centralize-bot-workflows design doc): pass the connection details through a file instead of a credential-named env var. - engineer-bot.yml: a new step writes host/http_path/token/user/catalog to $RUNNER_TEMP/e2e-connection.json (built with jq so the secret is never shell- interpolated; chmod 600) and exports its PATH to the author step as DATABRICKS_TEST_CONFIG_FILE — a name env_scrub deliberately preserves. - conftest.py: each connection fixture now falls back to that file when its env var is absent (env var still wins). Normal CI and local dev leave the path var unset, so the file dict is empty and behavior is byte-for-byte unchanged. Author phase only: the follow-up phase deliberately runs mocked tests/unit and forbids tests/e2e, so it needs no live token (left untouched). Co-authored-by: Isaac Signed-off-by: eric-wang-1990 <e.wang@databricks.com>
…#798) * Add OSV-Scanner-based security workflow Single workflow, single job, three triggers: - pull_request to main: fails on CVSS >= 7 findings only (HIGH/CRITICAL block merges; MED/LOW visible but non-blocking) - cron weekly (Sunday 00:00 UTC): reports ALL findings via email - workflow_dispatch: behaves like cron Mirrors the JDBC driver's security workflow (databricks-jdbc#1460) adapted for Python: - Reads poetry.lock natively via OSV-Scanner --lockfile (no separate SBOM tool needed) - Reuses the existing ./.github/actions/setup-jfrog composite action for parity with other workflows (the workflow functionally doesn't need JFrog since OSV reads the lockfile directly, but keeping the composite action preserves the established pattern) - Suppressions in osv-scanner.toml ([[IgnoredVulns]] schema) The workflow is not yet wired into branch protection. Day-one scan against current main surfaces 14 HIGH / 10 MED / 1 LOW (25 total) -- concentrated in cryptography, urllib3, pyjwt, pyarrow, requests, black, pytest, python-dotenv, idna. These will be addressed by a follow-up dep-bump PR. Co-authored-by: Isaac Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com> * Harden OSV gate (fail-closed) + refresh lockfile to clear all CVEs Port the fail-closed hardening from the Go (#362) and Node (#388) OSV workflows, and refresh poetry.lock so the gate passes with zero suppressions. securityScan.yml hardening (was fail-open in three places): - Capture osv-scanner's exit code; tolerate only 0/1 and fail closed on any other code (network error, corrupt binary) instead of masking it with `|| true`. - Validate the output is well-formed JSON with a .results array before parsing, so a truncated/partial scan fails closed rather than parsing to zero findings. - Resolve empty group max_severity via a cvss_num fallback to an UNKNOWN sentinel (using `try (x|tonumber) catch null`, not `tonumber?`), so a scoreless finding can never sort to 0 and sail past the CVSS>=7 gate. UNKNOWN always blocks (PyPA advisories carry CVSS; a scoreless finding is a GHSA-only/malware advisory). - Integer-count guards fail closed on parse failure. - Drop per-repo SMTP email in favor of artifact upload for the planned cross-repo collator (parity with Go/Node). CVE clearing WITHOUT forcing dependency floors: - Bump the Python floor to ^3.10. The CVE-fixed cryptography (>=46) and pyjwt (>=2.12) require Python >=3.10 upstream, so a single CVE-clean lockfile cannot span 3.8/3.9. This is the only breaking change. - All runtime dependency pins are UNCHANGED (thrift ~=0.22.0, urllib3 >=1.26, requests ^2.18.1, pyjwt ^2.0.0, pyarrow floors). The existing constraints already ALLOW the CVE-free versions; the refreshed lock simply resolves to them (urllib3 2.7.0, cryptography 49.0.0, pyarrow 23.0.1, requests 2.34.2, pyjwt 2.13.0, idna 3.18, python-dotenv 1.2.2). Customers do not need us to relax or raise any pin to become CVE-free. - thrift stays ~=0.22.0 (no known advisory; the <0.23 cap avoids the ES-1960554 DBR-LTS install break). - Bump dev-only black ^22 -> ^26 and pytest ^7 -> ^9 to clear their advisories (never shipped in the wheel); reformat src with black 26. Result: OSV-Scanner v2.3.8 reports 0 findings on the refreshed lock; osv-scanner.toml needs no suppressions. Co-authored-by: Isaac Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com> * ci: drop Python 3.9 from CI matrices to match the ^3.10 floor The pyproject floor is now ^3.10, so 3.9 legs can no longer `poetry install` (^3.10 is unsatisfiable on a 3.9 interpreter) and would fail. Remove "3.9" from every unit-test / lint / type-check / pyarrow / kernel matrix in code-quality-checks.yml and warm-deps-cache.yml, and drop the now-moot 3.9-kernel exclude in the warm-deps cache. Co-authored-by: Isaac Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com> * Document thrift CPE-gap accurately + add weekly NVD-CPE thrift watch Corrects the earlier "thrift ~=0.22.0 (no known advisory)" wording, which was wrong: thrift 0.22.0 IS affected by open Apache Thrift advisories (CVE-2025-48431 + the CVE-2026-41602..41636 set, all fixed in 0.23.0). Why we still hold at ~=0.22.0 and accept them: - Apache Thrift is one monorepo shipping ~20 language libraries; the PyPI `thrift` package is built only from lib/py. Each of these CVEs is in a NON-Python binding -- verified against the upstream oss-security advisories: Node.js (41636), Go (41602), c_glib/C (48431), Java (41603), Swift (41604, 41605). None touches the Python code paths we ship. - The only fix (0.23.0) is the version that caused SEV0 ES-1960554 on DBR-LTS old setuptools, so we cannot take it until a build-safe thrift ships (THRIFT-6067). Why the OSV gate doesn't flag it (and why that is NOT proof Python is safe): - These CVEs are in OSV with `affected[].package = null` -- only a GIT/CPE coordinate (cpe:2.3:a:apache:thrift), no PyPI/npm/Go package entry. OSV and Dependabot both match by package purl, so they return nothing for PyPI thrift. This is a coordinate blind spot, independent of whether Python is affected -- a FUTURE Python-affecting thrift CVE filed the same way would also be missed. Mitigation: a supplementary NVD-CPE thrift watch in securityScan.yml, scheduled/manual only (never PR; NVD rate limits). It lists all apache:thrift CVEs affecting the locked version in the weekly summary and hard-fails if any description names Python. Scoped to thrift alone because an audit of all three drivers' full dependency sets found thrift is the only dep with this purl-vs-CPE gap (Go/Node already ship the fixed 0.23.0). The Python-detection is a heuristic (description must say python/lib/py); the full list is always surfaced for human review. Co-authored-by: Isaac Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com> * fix(types): resolve 2 mypy errors surfaced by the refreshed mypy The lockfile refresh floats mypy (^1.10.1) up to 1.20.2, which is stricter and flags two pre-existing latent type issues that the older mypy missed: - auth/oauth.py:48 — is_expired() returned `exp_time and (...)`, whose value is `Any | None` (the exp claim) when falsy, not bool, violating the `-> bool` annotation. Use `exp_time is not None and (...)` so the return is a real bool and the None-exp case is explicit. - auth/retry.py:248 — the command_type setter was annotated `value: CommandType`, but the getter returns `Optional[CommandType]` and __private_init__ assigns an `Optional[CommandType]`. Widen the setter to `Optional[CommandType]` to match the getter and actual usage. Both are type-annotation-only changes; no runtime behavior change. Co-authored-by: Isaac Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com> * fix(tests): declare pytz explicitly (pandas 3.0 dropped it as a hard dep) tests/unit/test_parameters.py imports pytz directly. It previously arrived transitively via pandas, but the refreshed lock resolves pandas 3.0.3 on Python >=3.11, and pandas 3.0 removed pytz from its required dependencies (it is now only a pandas extra). That broke test collection on 3.11+ with `ModuleNotFoundError: No module named 'pytz'`. Add pytz as an explicit dev dependency so the test suite no longer relies on pandas's transitive graph, which differs across the Python matrix. Co-authored-by: Isaac Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com> * Bump thrift to ~=0.24.0 (CVE-clean + DBR-LTS-safe via wheels) thrift 0.24.0 shipped (THRIFT-6067) after this PR was opened. It is the first release that both clears the open Apache Thrift CVEs (CVE-2025-48431 + the CVE-2026-41602..41636 set, all fixed in 0.23.0) AND is safe to install on DBR LTS: unlike the yanked 0.23.0 (sdist-only, setup.py sys.exit(0) → SEV0 ES-1960554), 0.24.0 ships prebuilt manylinux2014 wheels (cp310-cp314) + macOS/musl/Windows, so pip installs a wheel and never runs setup.py -- the ES-1960554 build break cannot trigger. Gated on the DBR LTS Install CI check. Supersedes the earlier "hold at ~=0.22.0 and accept the thrift CVEs as non-Python-binding" stance now that a build-safe fixed thrift exists. The supplementary NVD-CPE thrift watch is no longer needed and is removed. Lockfile regenerated: thrift 0.24.0; all other CVE deps unchanged and clean. OSV-Scanner: 0 findings. Co-authored-by: Isaac Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com> * Remove the supplementary thrift NVD-CPE watch (obsolete after 0.24.0) The weekly NVD-CPE thrift watch existed only because we were accepting the thrift CVEs on the 0.22.0 pin (filed against the CPE, not the PyPI purl, so the OSV gate couldn't see them). With thrift bumped to 0.24.0 those CVEs are actually cleared, so the watch has nothing left to guard. Remove it; the normal OSV gate + version pin handle any future thrift CVE. Co-authored-by: Isaac Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com> --------- Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
…check (#879) * ci: dispatch coverage-fanout to databricks-driver-test on merged source PRs Wires databricks-sql-python into the multi-language coverage fan-out. When a PR merges to main and touched driver source (a file under src/), dispatch a `coverage-fanout` repository_dispatch to databricks/databricks-driver-test. Its coverage-fanout-tracker.yml then opens a tracking issue and runs the language-agnostic fan-out (a spec authored from this PR's diff, conformed across every driver). - Adds `closed` to the pull_request trigger types; the new trigger-coverage-fanout job gates on pull_request.merged == true. - Source-path filter (src/): docs/CI/test-only merges don't warrant a full fan-out. - Reuses the existing INTEGRATION_TEST App token (scoped to driver-test) + the same peter-evans/repository-dispatch pin adbc-drivers/databricks uses. - Tightens skip-integration-tests-pr's guard to exclude `closed` so it doesn't re-stamp a check on merged PRs. Co-authored-by: Isaac Signed-off-by: Eric Wang <e.wang@databricks.com> * ai: apply changes for #879 (1 review thread) Addresses: - #3641231149 at .github/workflows/trigger-integration-tests.yml:468 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com> * ai: apply changes for #879 (1 review thread) Addresses: - #3641618444 at .github/workflows/trigger-integration-tests.yml:459 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com> * ci: address review — narrow minted token + fix pin version comments Second-round peco-review-bot findings on the coverage-fanout sender (the engineer-bot applied round 1 — base.ref guard + job permissions — directly on this branch; this adds the remaining two): - Narrow the minted App token with `permission-contents: write` (coverage_fanout only needs repository_dispatch → contents:write), matching the defense-in-depth the other dispatch jobs use — a leaked token can only fire dispatches. - Restore the version tag in two action-pin comments (`# pinned` → `# v7.1.0` github-script, `# v3.0.0` create-github-app-token) per repo convention. Co-authored-by: Isaac Signed-off-by: Eric Wang <e.wang@databricks.com> * ai: apply changes for #879 (1 review thread) Addresses: - #3641857632 at .github/workflows/trigger-integration-tests.yml:479 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com> * ai: apply changes for #879 (1 review thread) Addresses: - #3641887238 at .github/workflows/trigger-integration-tests.yml:511 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com> * ai: apply changes for #879 (1 review thread) Addresses: - #3641913695 at .github/workflows/trigger-integration-tests.yml:513 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com> * ai: apply changes for #879 (1 review thread) Addresses: - #3641939652 at .github/workflows/trigger-integration-tests.yml:530 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com> * ai: apply changes for #879 (1 review thread) Addresses: - #3641958020 at .github/workflows/trigger-integration-tests.yml:540 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com> * ai: apply changes for #879 (1 review thread) Addresses: - #3641977677 at .github/workflows/trigger-integration-tests.yml:494 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com> * ai: apply changes for #879 (1 review thread) Addresses: - #3641995560 at .github/workflows/trigger-integration-tests.yml:476 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com> * ai: apply changes for #879 (1 review thread) Addresses: - #3642007200 at .github/workflows/trigger-integration-tests.yml:493 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com> * ci: fix dead driver-test workflow link in dispatch comment The "Integration tests triggered" PR comment (and two header comments) linked to python-proxy-tests.yml, which does not exist (404) — the receiver file is databricks-sql-python-proxy-tests.yml. Point to the correct workflow and note that the authoritative results are the "Python Proxy Tests / thrift" and "Python Proxy Tests / kernel" checks posted back on the PR. Co-authored-by: Isaac Signed-off-by: eric-wang-1990 <e.wang@databricks.com> * ai: apply changes for #879 (1 review thread) Addresses: - #3643591969 at .github/workflows/trigger-integration-tests.yml:455 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com> * ci: post single "Python Integration Tests" check (unify with go/nodejs) Python was the last multi-backend driver posting per-mode checks (`Python Proxy Tests / thrift` + `/ kernel`). The new standalone receiver in databricks-driver-test (databricks-python-integration-tests.yml) fans out both backends internally and reports ONE aggregated check, matching go/nodejs. Collapse the five `MODES=['thrift','kernel']` two-check loops (skip / label auto-pass / dispatch-fail / merge-queue auto-pass / merge-queue-fail) into a single `Python Integration Tests` check, add `proxy_mode: replay` to both dispatch payloads for parity with the other senders, and fix the PR-comment link to the new receiver workflow. The dispatch itself was already a single `python-pr-test` (no per-mode fan-out on the sender). Follow-up (branch protection, done separately): swap the required-checks list — remove `Python Proxy Tests / thrift` and `/ kernel`, add `Python Integration Tests`. Sequenced after this + the receiver land to avoid a deadlock. Co-authored-by: Isaac Signed-off-by: eric-wang-1990 <e.wang@databricks.com> * ai: apply changes for #879 (1 review thread) Addresses: - #3643898716 at .github/workflows/trigger-integration-tests.yml:484 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com> --------- Signed-off-by: Eric Wang <e.wang@databricks.com> Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com> Signed-off-by: eric-wang-1990 <e.wang@databricks.com> Co-authored-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>
…#882) * ci(bots): add daily retrospective (rolling learning PR) + bump engine Adds the daily learning-extraction flow (was not enabled on this repo) on the current engine d05dcb1, which maintains ONE rolling learning PR on a stable branch (ai/learning-pr). - engineer-bot-learning.yml: daily cron + workflow_dispatch; PAT-free via the existing bot-prelude; actions:read for Track B (author-run mining). - .bot/config.yaml: add retrospective: block (engine-owned batch prompt, no override) and author.knowledge_log to close the loop (author reads what the retrospective writes). - Seed .claude/knowledge/learning-log.md. - Bump bot-prelude engine-ref default 5368de7 → d05dcb1. Co-authored-by: Isaac Signed-off-by: eric-wang-1990 <e.wang@databricks.com> * ai: apply changes for #882 (1 review thread) Addresses: - #3653811351 at .github/workflows/engineer-bot-learning.yml:78 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com> * ai: apply changes for #882 (1 review thread) Addresses: - #3654123624 at .github/workflows/engineer-bot-learning.yml:75 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com> * ci: post Python Integration Tests check as the driver-test app (fix app-pinned gate) The required 'Python Integration Tests' check is pinned in the main ruleset to the driver-test app (driver-integration-test, integration_id 2731531). But the PR-side placeholder was posted via github.token, so it landed as github-actions — a DIFFERENT check context that does NOT satisfy the app-pinned gate. Result: the required check sat unfulfilled and PRs stayed BLOCKED even though a green 'Python Integration Tests' appeared in the list (e.g. #882). Mirror databricks-sql-go #426 / databricks-sql-nodejs #459: - Add skip-checks-reporter.yml, triggered by workflow_run on 'Trigger Integration Tests'. It runs in the base-repo context with secret access even for fork-triggered runs, mints the INTEGRATION_TEST_APP token, and posts 'Python Integration Tests'=success on every PR head — forks included — as the driver-test app, so the app-pinned gate is satisfied and every PR auto-enqueues. - Remove the inline skip-integration-tests-pr github.token stub (superseded; it posted under the wrong app and 403'd on forks entirely). - In merge-queue-python, mint the app token unconditionally and post the auto-pass (no-driver-change) and dispatch-failure checks with IT — the merge_group path isn't covered by the reporter, and a github.token check there likewise can't satisfy the pinned gate (queue would hang). The real result was already posted back by driver-test as the app; this aligns the no-op paths. The labeled-PR preview's inline checks stay on github.token (cosmetic; the reporter owns the pinned gate on pull_request events), matching go/nodejs. Co-authored-by: Isaac Signed-off-by: eric-wang-1990 <e.wang@databricks.com> * ai: apply changes for #882 (1 review thread) Addresses: - #3654256027 at .github/workflows/skip-checks-reporter.yml:39 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com> * ci: post PR-open skip check inline for internal PRs (fix waiting-for-status hang) Follow-up to the previous commit. Removing the inline PR-open poster in favour of the workflow_run reporter created a bootstrap gap: workflow_run workflows only run from the DEFAULT-branch copy, so on this PR (and any introducing PR) the reporter can't fire and NOTHING posts 'Python Integration Tests' on the PR head — the required check sits 'Expected — Waiting for status to be reported' forever. Fix: split the placeholder by PR origin, both posting as the driver-test app (the identity the ruleset pins the required check to): - Internal PRs: restore skip-integration-tests-pr in trigger-integration-tests.yml, now minting the INTEGRATION_TEST_APP token (not github.token) and self-guarded to head repo == base repo. Posts from the PR branch immediately — no label, no workflow_run dependency. This is what unblocks internal PRs like this one. - Fork PRs: skip-checks-reporter.yml (workflow_run) now self-guards to fork runs (head repo != base repo), since a fork's pull_request run can't mint secrets inline. The two guards are mutually exclusive — no double-post. Behaviour matches the intent: skipped placeholder on PR open, real run in the merge queue (and label preview). The merge-queue app-token posts from the prior commit are unchanged. Co-authored-by: Isaac Signed-off-by: eric-wang-1990 <e.wang@databricks.com> * ai: apply changes for #882 (1 review thread) Addresses: - #3654339217 at .github/workflows/engineer-bot-learning.yml:34 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com> * ci: clarify merge-queue mint-failure is fail-closed (review r3654295072) Soften the overstated 'the mint always succeeds here' comment and document the mint-failure case explicitly: it is uncloseable in-workflow (only the pinned app can post to the pinned context; a github.token fallback lands on a different context and neither satisfies nor fails the gate), but it is fail-CLOSED — the merge queue's check_response_timeout evicts an entry whose required check never reports, and the mint failure shows as a red run. No code change; comment only. Co-authored-by: Isaac Signed-off-by: eric-wang-1990 <e.wang@databricks.com> * ai: apply changes for #882 (1 review thread) Addresses: - #3654417922 at .github/workflows/trigger-integration-tests.yml:366 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com> * ci: refresh check state (re-post required checks on a clean head) No file changes. The prior head accumulated many check-suites across repeated force-push rebases, and GitHub's mergeability rollup stopped surfacing the required 'Kernel E2E' context even though a valid github-actions success check-run existed on the commit — leaving the PR unable to enter the merge queue ('Kernel E2E — Expected — Waiting for status to be reported'). Pushing a fresh non-force commit gives a clean check-suite set so skip-kernel-e2e-pr re-posts Kernel E2E into the current suite and the rollup picks it up. Co-authored-by: Isaac Signed-off-by: eric-wang-1990 <e.wang@databricks.com> --------- Signed-off-by: eric-wang-1990 <e.wang@databricks.com> Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com> Co-authored-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>
chore(bots): bump engine pin d05dcb11 -> 654a31d0 Move all bots to engine main (654a31d0), 30 commits ahead of the current pin. This repo already runs the modern daily-cron retrospective + closed loop; this brings the pinned engine in line with the sibling driver repos (kernel + the go/nodejs/jdbc/odbc learning PRs), whose engine was bumped to the same SHA. The main addition since d05dcb11 is per-bot model selection (default opus-4.8). SDK/CLI locked versions (0.2.102 / 2.1.61) are unchanged at 654a31d0, so this is a SHA-only bump — no install-bot-engine edit needed. Co-authored-by: Isaac Signed-off-by: eric-wang-1990 <e.wang@databricks.com>
) fix(learning): correct MODEL_ENDPOINT to the concrete serving-endpoint form The learning workflow set MODEL_ENDPOINT to `.../serving-endpoints/anthropic/invocations`. That is NOT translated the way the old comment claimed: sdk_agent.translate_endpoint has an early-return for URLs already containing `/serving-endpoints/anthropic`, which keeps the trailing `/invocations` — so the CLI appends `/v1/messages` and calls `.../serving-endpoints/anthropic/invocations/v1/messages`, which the gateway rejects with `400 Unsupported native API path`. Proof: databricks-sql-python's learning cron (same endpoint form) has failed every scheduled run with exactly this 400. The reviewer-bot flagged it on the sibling PRs (Low), and it is the real cause. Use the concrete `.../serving-endpoints/databricks-claude-opus-4-8/invocations` form that reviewer-bot.yml / engineer-bot.yml already use successfully: translate_endpoint strips `<model>/invocations` to the `.../serving-endpoints/ anthropic` base the CLI needs. Comment corrected to explain the trap. Co-authored-by: Isaac Signed-off-by: eric-wang-1990 <e.wang@databricks.com>
* feat(kernel): forward identity federation client ID * docs: preserve existing kernel comment * test(kernel): cover federation client ID forwarding
* fix(kernel): honor cursor row limit * test(kernel): cover exact row limit boundary
…040) (#914) * fix(kernel): forward full OAuth U2M app bundle into kernel (PECOBLR-4040) On the use_kernel path, OAuth U2M forwarded only whatever the caller explicitly set, sending a bare oauth-u2m otherwise. Since PECOBLR-4039 changed the kernel core default U2M app to databricks-sql-connector / sql offline_access / port 8030, a bare U2M connection authenticated as the wrong identity. The connector is an OVERRIDE of the kernel default, so it now forwards its full coupled bundle (client_id + oauth_scopes + redirect_port). Each field falls back to the connector's registered databricks-sql-python (or azure) default from the existing PYSQL_OAUTH_* constants, giving parity with the Thrift path. Explicit caller overrides and identity_federation_client_id forwarding are preserved. Co-authored-by: Isaac Signed-off-by: eric-wang-1990 <e.wang@databricks.com> * ai: apply changes for #914 (1 review thread) Addresses: - #3799256419 at src/databricks/sql/backend/kernel/auth_bridge.py:266 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com> * ai: apply changes for #914 (1 review thread) Addresses: - #3799336213 at src/databricks/sql/backend/kernel/auth_bridge.py:275 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com> * ai: apply changes for #914 (1 review thread) Addresses: - #3799379619 at src/databricks/sql/backend/kernel/auth_bridge.py:283 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com> * fix(kernel): don't handle azure-oauth U2M yet; reject it (PECOBLR-4120) Azure AD U2M can't work through the kernel today: the kernel resolves OAuth endpoints only from the workspace-native OIDC config and has no Azure AD path, so the Thrift azure-oauth flow (AAD token endpoint + /user_impersonation scope) cannot be reproduced. Rather than forward an azure bundle that authenticates against the wrong endpoints, reject auth_type='azure-oauth' up front with a clear NotSupportedError pointing at the Thrift backend. The kernel U2M path now handles databricks-oauth only. Azure support is tracked by PECOBLR-4120. Also fixes the stale scope tests the prior review left red: scopes are hardcoded to PYSQL_OAUTH_SCOPES for Thrift parity (not caller- overridable), and the tests now assert that. Co-authored-by: Isaac Signed-off-by: eric-wang-1990 <e.wang@databricks.com> * ai: apply changes for #914 (2 review threads) Addresses: - #3799622645 at src/databricks/sql/backend/kernel/auth_bridge.py:254 - #3799650240 at src/databricks/sql/backend/kernel/auth_bridge.py:300 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com> * ai: apply changes for #914 (1 review thread) Addresses: - #3799717722 at src/databricks/sql/backend/kernel/auth_bridge.py:275 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com> * ai: apply changes for #914 (1 review thread) Addresses: - #3799762589 at src/databricks/sql/backend/kernel/auth_bridge.py:261 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com> * ai: apply changes for #914 (1 review thread) Addresses: - #3799812091 at src/databricks/sql/backend/kernel/auth_bridge.py:258 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com> * fix(kernel): forward redirect_ports list to kernel U2M (PECOBLR-4144) databricks-sql-kernel #257 landed: the pyo3 Session now takes redirect_ports (a list) and no longer accepts the single redirect_port kwarg. Update the kernel auth bridge to emit redirect_ports for databricks-oauth U2M, forwarding the databricks-sql-python app's FULL registered port list (PYSQL_OAUTH_REDIRECT_PORT_RANGE, 8020-8024) so the kernel binds the first free port — busy-port fallback, matching the Thrift DatabricksOAuthProvider. A custom client_id + explicit port pins that single port ([port]). Bump KERNEL_REV to the merged #257 commit (45a0d6a) so kernel-e2e builds against the kernel that exposes redirect_ports. Tests updated. Co-authored-by: Isaac Signed-off-by: eric-wang-1990 <e.wang@databricks.com> --------- Signed-off-by: eric-wang-1990 <e.wang@databricks.com> Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com> Co-authored-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>
* docs: add connection parameters reference (Thrift/SEA/Kernel) Add docs/connection-parameters.md documenting every sql.connect() keyword argument: type, default, per-backend support (Thrift / SEA / Kernel), and meaning. Verified against Connection.__init__, the per-backend clients, and the shared HTTP/auth layer at connector 4.4.0. Linked from the README Documentation section. Co-authored-by: Isaac Signed-off-by: eric-wang-1990 <e.wang@databricks.com> * docs: rework connection parameter reference in Thrift-vs-Kernel table format Drop the SEA column (backend is being deprecated) and restructure the doc to match the databricks-sql-nodejs connection-parameter reference (#457): a two-backend Thrift/Kernel table per section with a ✅/❌/⚠️ /— legend, a 'Sources of truth' list, and a 'Summary of gaps'. Co-authored-by: Isaac Signed-off-by: eric-wang-1990 <e.wang@databricks.com> * docs: correct kernel auth/query_tags rows against origin/main - credentials_provider is rejected on the kernel path (NotSupportedError), not supported: mark Kernel ❌. - Per-statement query_tags IS supported on the kernel via set_query_tags: drop the stale 'not supported' caveat. - azure_* fields are the Entra M2M gap; note azure-oauth U2M browser flow does work on the kernel. - Fix a backwards Summary-of-gaps entry (oauth_client_secret/oauth_scopes are kernel-supported; the real Thrift-only auth gap is credentials_provider). Co-authored-by: Isaac Signed-off-by: eric-wang-1990 <e.wang@databricks.com> * ai: apply changes for #913 (3 review threads) Addresses: - #3799244504 at docs/connection-parameters.md:6 - #3799383910 at docs/connection-parameters.md:90 - #3799696042 at docs/connection-parameters.md:72 Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com> * docs: clarify oauth_redirect_port is client-id-gated on both backends The oauth_redirect_port row implied the kernel path treats the port differently from Thrift. It doesn't: auth.py:127-130 (Thrift) and the kernel auth_bridge use the identical gate — an explicit port is only honored when a custom oauth_client_id is also supplied; otherwise the full 8020-8024 range is used with first-free-port binding. Reword the note to describe this shared behavior (both backends), keeping the existing checkmark/checkmark support columns. Co-authored-by: Isaac Signed-off-by: eric-wang-1990 <e.wang@databricks.com> * docs: move connection reference to root as CONNECTION_PARAMETERS.md Match the databricks-sql-go layout (databricks/databricks-sql-go#445): put the reference at the repo root as CONNECTION_PARAMETERS.md and link it from README's docs section, instead of docs/connection-parameters.md. Retitle to the plural 'Connection parameters reference' and fix the relative proxy link (proxy.md -> docs/proxy.md) now that the file sits one level up. Co-authored-by: Isaac Signed-off-by: eric-wang-1990 <e.wang@databricks.com> * docs: retitle CONNECTION_PARAMETERS and fix relative proxy link Plural title (matches databricks-sql-go) and repoint the in-table proxy link proxy.md to docs/proxy.md now that the file lives at the repo root. Also update the README reference path. Co-authored-by: Isaac Signed-off-by: eric-wang-1990 <e.wang@databricks.com> --------- Signed-off-by: eric-wang-1990 <e.wang@databricks.com> Signed-off-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com> Co-authored-by: peco-engineer-bot[bot] <peco-engineer-bot[bot]@users.noreply.github.com>
The SEA backend (use_sea=True) has feature gaps — notably it does not support positional (`?`) parameter binding, which causes HTTP 400s against RT/Lakehouse warehouses. Rather than partially patch SEA, mark it deprecated and steer users to the supported kernel backend (use_kernel=True), which is SEA-native and handles positional/named parameter binding. - Emit a warning at SeaDatabricksClient construction pointing users to use_kernel=True + the `[kernel]` extra. - Document use_sea as deprecated/incomplete in the connect() docstring, and refresh the stale use_kernel docstring (it now ships on PyPI via the `[kernel]` extra and supports parameter binding). - Flag the SEA example harness and the CONTRIBUTING backend table as deprecated. SEA stays functional (RT warehouses refuse Thrift, so it can't simply be rerouted) and is slated for eventual removal. Co-authored-by: Isaac Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
* fix(kernel): pass cursor row limit to kernel * test(kernel): cover zero row limit
* feat(kernel): support JWT private-key M2M auth on use_kernel=True Route JWT private-key client-assertion auth (RFC 7523) through the kernel backend. When the caller passes `oauth_jwt_key_file` (+ `oauth_client_id` and `oauth_jwt_kid`, optional `oauth_jwt_passphrase` / `oauth_jwt_algorithm` / `oauth_scopes` / `token_url`), the bridge forwards them to the kernel's `auth_type="oauth-m2m-jwt"`, which signs a short-lived assertion with the private key instead of sending a client secret and owns the token lifecycle. - auth_bridge.py: new JWT branch (checked before shared-secret M2M and PAT, since a private-key file is unambiguous JWT M2M intent); mutually exclusive with oauth_client_secret / credentials_provider; requires client_id + kid. - session.py: forward the new oauth_jwt_* / token_url kwargs into the kernel auth options. - tests: 9 unit tests covering routing, precedence, validation, and ambiguity guards. Verified end-to-end: `SELECT 1` via use_kernel=True against an Azure Databricks warehouse, authenticated by Entra ID against the service principal's registered public certificate. Signed-off-by: Rahul Singhal <rahul.singhal@databricks.com> * fix(kernel): annotate kwargs at first assignment to satisfy mypy The JWT branch introduced an earlier untyped `kwargs =`, so mypy flagged the M2M branch's `kwargs: Dict[str, Any]` as a redefinition. Move the annotation to the first (JWT) assignment. Signed-off-by: Rahul Singhal <rahul.singhal@databricks.com> * fix(kernel): guard JWT M2M + databricks-oauth auth_type; renumber comments Address peco-review-bot review on #921: - Medium: add the missing ambiguity guard for oauth_jwt_key_file + auth_type="databricks-oauth" (U2M intent), mirroring the existing shared-secret M2M + U2M guard. Fails loudly rather than silently resolving to one flow. Covered by a new unit test. - Low: renumber the inline resolution-order comments (PAT→3, U2M→4, creds→5, else→6) to match the docstring after the JWT branch insert. Signed-off-by: Rahul Singhal <rahul.singhal@databricks.com> * docs: document JWT private-key M2M params in CONNECTION_PARAMETERS.md Address Eric's review on #921: add rows for oauth_jwt_key_file / oauth_jwt_kid / oauth_jwt_passphrase / oauth_jwt_algorithm / token_url to the connection-parameter reference (all kernel-only). Signed-off-by: Rahul Singhal <rahul.singhal@databricks.com> * security: suppress unreachable cryptography PKCS#7 advisory (osv-scanner) Security Scan flagged cryptography@49.0.0 CVE-2026-69247 (GHSA-g6cj-pr64-35w5 / PYSEC-2026-3552, CVSS 8.2) — a Bleichenbacher-style oracle in PKCS#7 EnvelopedData / S-MIME decryption. `cryptography` is a transitive dep (PyJWT[crypto] / oauthlib) used only for OAuth token signing/verification; the connector never decrypts PKCS#7 / S-MIME, so the vulnerable path is never invoked. Pre-existing on main (transitive, not introduced by this PR). Suppress both OSV ids with justification; a clean bump awaits PyJWT/oauthlib floors pulling cryptography>=50. Signed-off-by: Rahul Singhal <rahul.singhal@databricks.com> * fix(kernel): forward token_url on shared-secret M2M too (JDBC parity) Address peco-review-bot: the CONNECTION_PARAMETERS.md row said token_url "applies to shared-secret M2M too", but the bridge only forwarded it on the JWT branch — so an oauth_client_id + oauth_client_secret + token_url connection silently dropped token_url. token_url is an auth-method-agnostic token-endpoint override: JDBC's OAuth2ConnAuthTokenEndpoint is consumed by the client-secret M2M provider, the JWT provider, and the refresh provider alike; the Node driver also forwards it on both branches; and the kernel's oauth-m2m auth_type accepts it (pyo3). Forward it on the shared-secret M2M branch so code matches the doc and the other drivers. Adds a unit test. Signed-off-by: Rahul Singhal <rahul.singhal@databricks.com> --------- Signed-off-by: Rahul Singhal <rahul.singhal@databricks.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
20 participants
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.
TEst