ci: add label-gated proxy-test dispatch to databricks-driver-test#799
Conversation
msrathore-db
left a comment
There was a problem hiding this comment.
LGTM. Just sanitise the title
Wires this repo into the proxy-based integration test infrastructure
in databricks/databricks-driver-test. Mirrors the canonical pattern
established in adbc-drivers/databricks (5-job workflow, not the
simpler ODBC variant).
How it works:
- On a normal PR event (open / push / reopen / non-IT label) we
post a `success` Python Proxy Tests check immediately so the
required check doesn't block the PR. The real tests run in the
merge queue.
- When a maintainer adds the `integration-test` label we dispatch
the suite as a preview — useful for catching regressions before
merge queue time. Includes path-filter detection: if only
docs / tests/unit / etc. changed, auto-pass instead of dispatching.
- Pushing new commits auto-removes the label so a subsequent
labelled run requires a fresh maintainer review.
- On the `merge_group` event the suite runs as the real required
gate. Only PRs whose tests dispatch (or auto-pass when no driver
files changed) can proceed to `main`.
Security:
- Label-gated PR preview: only maintainers can apply
`integration-test`. Fork PRs are not auto-trusted.
- Auto-removed on synchronize: pushing new commits clears the
label, forcing maintainer re-review before tests re-run.
- PR titles are sanitized before interpolation into the dispatch
JSON payload (prevents quote/newline-based JSON injection).
- Dispatch failures post a `failure` check-run via a separate
public-repo App token so the gate never silently turns green
when the dispatcher itself broke.
Operational follow-ups (outside this PR):
1. Create the `integration-test` label in this repo (already done).
2. Install `INTEGRATION_TEST_APP_ID` / `INTEGRATION_TEST_PRIVATE_KEY`
secrets for the GitHub App with write access to
databricks/databricks-driver-test.
3. Enable merge queue on `main` branch protection AND mark
`Python Proxy Tests` a required status check. Until this is done
the merge-queue job is dead code and ITs run only on explicit
label.
Addresses review comments on databricks-sql-python#799 (JSON
injection + sparse merge-queue payload) and adopts the
adbc-drivers/databricks canonical pattern wholesale.
Co-authored-by: Isaac
Signed-off-by: Vikrant Puppala <vikrant.puppala@databricks.com>
6d7f15f to
86c8d27
Compare
|
Thanks for the review @msrathore-db — both comments uncovered a deeper issue. Investigating your point about the merge-queue payload prompted me to look at the canonical pattern in adbc-drivers/databricks (the established gold standard for cross-repo IT dispatch in this org). It turns out the original version of this PR — which I'd copied from I've rewritten the workflow from scratch to match the canonical 4-job pattern (the ADBC version has 5 — extra one is for a second driver in the same repo). What's new (vs the original 2-job version)
Both your comments addressed
Operational follow-ups (outside this PR)For this to deliver value, after merge someone needs to:
The Until step 1+2 are done, the merge-queue job is dead code and ITs run only on explicit label — same effective behaviour as the original version. The label path is fully usable in that interim state. Re-pushed via |
Summary
Wires this repo into the proxy-based integration test infrastructure in databricks/databricks-driver-test. Mirrors the pattern used by databricks-odbc#55 and the ADBC drivers (C# / Rust).
The driver-test repo already runs proxy-based integration tests against the canonical C# implementation on every PR to it. With this change, the same harness can be triggered against this repo's PRs:
integration-testlabel.python-pr-testevent todatabricks/databricks-driver-testwith the PR number + head SHA.pip install git+https://...@<sha>, runs the Python proxy test suite (databricks-driver-test#329,tests/python/, ~59 tests in Phase 1) against the pecotesting warehouse with mitmproxy in the loop, and posts a check run back to this PR.Security
integration-test.synchronize: pushing new commits clears the label, forcing maintainer re-review before tests re-run. A comment is posted explaining what happened.merge_groupis trusted at that point — dispatches automatically.The dispatch token comes from a GitHub App (
INTEGRATION_TEST_APP_ID/INTEGRATION_TEST_PRIVATE_KEY), not a personal PAT, so it can be scoped to justdatabricks-driver-testand rotated independently.Companion PRs
tests/python/scaffolding + Phase 1 tests + receiving CI workflow. Must merge before this PR is useful — without it, the dispatch goes nowhere.Test plan
integration-testlabel in this repo (one-time manual step — same as we did fordatabricks-odbc).python-pr-testdispatch appears in the driver-test Actions tab.Out of scope
This pull request and its description were AI-assisted by Isaac.