Skip to content

test(py): run the warehouse code against live Snowflake and Databricks - #434

Draft
jat255 wants to merge 13 commits into
mainfrom
jat255/whhh-live-warehouse-python
Draft

jat255 wants to merge 13 commits into
mainfrom
jat255/whhh-live-warehouse-python

Conversation

@jat255

@jat255 jat255 commented Oct 20, 2026 •

Copy link
Copy Markdown
Collaborator

This PR adds a pytest suite that runs the Python Snowflake and Databricks code against real warehouses, and a CI workflow that runs it (once each warehouse is configured). Up to now, every warehouse test in pkg-py used a fake that replays canned rows.

Refs #303

The tests check:

  • the real row shapes of the listing queries
  • how a real permission refusal is classified
  • what the session identity queries return
  • whether the SQL from the definition emitters computes the same result as DuckDB on the same rows

Each test skips when its warehouse is not configured, so it shouldn't block if credentials are missing.

Review notes

  • Configuration is environment variables. Their names are in tests/shared/live-warehouse-env.json, so the R suite reads the same ones (test(r): configure the live warehouse tests from the shared contract, and run them in CI #431). Databricks sign-in uses the Databricks SDK's unified auth (a token, a client secret, or GitHub OIDC). Snowflake uses a key pair, because Snowflake service users cannot sign in with a password.
  • scripts/provision-live-warehouse.py prints the SQL that creates the test tables from tests/shared/live-warehouse-rows.json. With --grant-to, it also gives the test principal read access and creates one table that the principal cannot read, for the refusal test.
  • The workflow runs on pushes to main and on manual dispatch, with credentials from the databricks-ci and snowflake-ci GitHub environments. Those environments do not exist yet. A backend with no variables set skips. A backend with only some set fails, so a misconfigured backend cannot pass by skipping every test.

Testing

The Python suite, ruff, and pyrefly pass. The live tests have not yet run against a real warehouse, because the Databricks workspace and the Snowflake service user are still being set up. Snowflake rejects the CI key with "JWT token is invalid", which is the expected reply while that user does not exist.

Agent-written notes

kata: whhh (milestone M10). Replaces #430, which reached main by a direct push and is reverted by #433.

Waiting on CloudOps CL-2949 (Databricks workspace and service principal) and rstudio/partnership-snowflake#4124 (the SVC_COMMONS Snowflake user). After those, the remaining steps are: create the two GitHub environments, run the provisioning SQL with --grant-to, and dispatch the workflow.

@jat255 jat255 added py Affects the Python implementation tests Related to testing or the test suite needs-manual-review Agent-created work that needs a human review labels Oct 20, 2026
@jat255
jat255 added this pull request to stack #435 October 20, 2026 22:55
@github-actions

Copy link
Copy Markdown
Contributor

Preview root: https://posit-dev.github.io/commons/pr-434/

Python site preview: https://posit-dev.github.io/commons/pr-434/py/

Built from the latest commit on this branch. The R links in it point at the published R site, which no pull request rebuilds.

@github-actions

github-actions Bot commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Preview deployed to Connect (dogfood.team.pct.posit.it): https://dogfood.team.pct.posit.it/connect/#/apps/d7a36cae-8f27-448b-a478-61b81fbe3942/draft/381993

Deployed from commit 43206af.

Base automatically changed from jat255/revert-430 to main October 20, 2026 22:57
@jat255
jat255 force-pushed the jat255/whhh-live-warehouse-python branch from d45a032 to bca3fff Compare October 20, 2026 22:57
@github-actions

github-actions Bot commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Preview deployed to Connect (connect.staging.pct.posit.it): https://connect.staging.pct.posit.it/connect/#/apps/ad662e1b-5048-4acc-9ad7-f9478c92274e/draft/3714

Deployed from commit 43206af.

…ronment

Every warehouse test in pkg-py drives a fake replaying canned rows, so
nothing had ever run a statement against a real warehouse. This adds the
other half.

Configuration is environment variables in both languages, with the names
held in tests/shared/live-warehouse-env.json so the R harness (which still
reads R options, and moves next) cannot drift from this one. Connection
variables are the vendor's own, which is what the Python clients and
odbc::databricks() already read, so one export block serves both languages
and the vendor CLIs. Without a token the Databricks connector runs the OAuth
browser flow, which is what makes an SSO-only account usable.

The fixture tables are the three the corpus dictionaries in
definition-export/valid/ describe, so definition-warehouse-sql.json can be
executed rather than only agreed on: each definition runs on the warehouse
and on the same rows in DuckDB, and the answers must match unless the record
carries a note, which is where an emitter documents a deliberate divergence.
Reading the exemption off the notes means a divergence cannot be claimed
without also being documented.

Rows live in the shared fixture, not in the provisioning SQL, because both
engines must hold the same data for the comparison to mean anything. One
definition is recorded as unrunnable: data-dict's own DuckDB translation of
interval(amount + 1, days) will not bind over a double, and amount has to be
fractional for `precise threshold` to compare anything.

test_live_warehouse_fixture.py covers what needs no warehouse, so CI checks
the rows, the loader and the generator rather than leaving them unexercised
until someone with credentials runs them.

Refs kata whhh.
Four review findings, all real.

Relation references were built by joining the identifier parts raw, which
breaks on both backends: `values` is a reserved word on Databricks, and
Snowflake folds an unquoted name to upper case while the fixture tables are
created under the authored spelling that the emitted definition SQL refers to
its columns by. SQLAlchemy's identifier preparer does not fix it, because it
quotes only where it judges quoting necessary and it judges both of those
safe. The backends' own readers quote unconditionally, so the harness now
uses them.

The description test built a source with no dictionary, and a warehouse
relation is only described when an authored table matched it, so every
relation's columns were empty and the assertion could never have passed. It
now passes the corpus dictionary the table was provisioned from, which also
puts the authored-to-warehouse column binding under test, and reads the
normalized `column` key rather than the raw reply's.

The denied-table probe built its TableId positionally. TableId takes the
table first and the catalog last, so a `catalog.schema.table` value arrived
reversed and the probe tested a relation nobody configured.

The driver dependencies are a uv group, not an extra, so the documented
`uv sync --extra` would not have installed them.

Refs kata whhh.
…in CI

Snowflake signs in with a key pair from SNOWFLAKE_PRIVATE_KEY_FILE, the
variable the Snowflake CLI and snowflakeauth read. Service users cannot use
passwords, so SNOWFLAKE_PASSWORD leaves the contract.

Databricks credentials now come from the SDK's unified authentication, which
resolves a token, a service principal's client secret, GitHub Actions OIDC,
or a CLI login from the environment. That replaces the harness's own choice
between a token and the browser flow, and it covers whichever credential the
CI service principal ends up with. GitHub OIDC needs databricks-sdk 0.55.

The new workflow runs each backend from its own GitHub environment on pushes
to main and on manual dispatch. It checks the contract's required variables
first: none set skips the job, and some set fails it, so a misconfigured
backend cannot pass by skipping every test.
… is refused

provision-live-warehouse.py takes --grant-to, naming the Snowflake role or
Databricks service principal the tests connect as. The SQL then grants it
usage on the namespace and SELECT on each fixture table, and creates
denied_orders, a copy of orders with no grant. Whoever runs the SQL owns the
copy and the test principal does not, which is the second principal the
denied-table fixture always needed. Without --grant-to the output is
unchanged and the denied table is still left to do by hand.

Databricks USE CATALOG stays with the catalog's owner, so the SQL says so
rather than granting it.
A principal that runs the grants owns denied_orders and can read it, which
turns the access-refusal test into a failure.
Its --grant-to option creates one owned by whoever runs the SQL, so the
contract no longer needs to call the fixture something provisioning cannot
finish.
The check job installs every extra but not the live-warehouse group, so a
static import of databricks.sdk.core failed pyrefly there, and would have
errored rather than skipped where the SDK is missing.
Splits the setup into three numbered sections and clarifies the denied-table
principal requirements: the SQL runner must differ from the test principal,
since owners can always read their own tables.
…ake owner role

The live-warehouse dependency group joins dev, so a plain `uv sync`
installs the drivers and the workflow no longer asks for the group.

On Snowflake the role that runs the provisioning SQL owns the denied copy,
whichever user signs in. The README, the script's docstring and the
contract's denied-table description now say to run it under a role other
than the one passed to --grant-to. The README's prose is soft-wrapped.
@jat255
jat255 force-pushed the jat255/whhh-live-warehouse-python branch from 57db6be to f66a845 Compare October 21, 2026 03:13
Snowflake accepts only constants in a VALUES list, so the ARRAY_CONSTRUCT and OBJECT_CONSTRUCT_KEEP_NULL literals for the survey table failed to compile. Snowflake inserts now use INSERT ... SELECT ... UNION ALL. Databricks keeps VALUES.
…e live tests

The comparison run read the warehouse and DuckDB in different session time zones, so a date cast to a timestamp, or compared with a zone-less literal, differed by the offset. Both sessions now use UTC.

The describe test compared the fixture's bare table names with the source's qualified labels, and looked for the survey table in lower case after folding the label to the warehouse's case.
The DuckDB reference reads in UTC, so the Databricks session has to as well for temporal answers to compare. Not yet run against a Databricks workspace.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-manual-review Agent-created work that needs a human review py Affects the Python implementation tests Related to testing or the test suite

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant