Run pull-request AWS tests on the newest Python version only - #863
laughingman7743 wants to merge 2 commits into
Conversation
The changes job now also selects the Python versions: pull requests test the newest supported version, and the weekly schedule and manual dispatch test every version. test-suite.yaml takes the versions as a required python-versions input instead of a fixed matrix. Closes #850 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
| PR_NUMBER: ${{ github.event.pull_request.number }} | ||
| # Every supported version, oldest first; keep in sync with the | ||
| # pyproject.toml classifiers, which the Release workflow checks. | ||
| PYTHON_VERSIONS: '["3.10", "3.11", "3.12", "3.13", "3.14"]' |
There was a problem hiding this comment.
Self-review round one (implementation behavior): CLEAN
Base 531185619465b809331a9a8cf2083884365de193, head a2eb20acf406a0e603ba37b562f1878c562e2c86.
Covered:
.github/workflows/test.yaml.github/workflows/test-suite.yamldocs/testing.md
Checked:
- Version selection.
pull_request→jq -c '[last]'→["3.14"].- Other events → the full list, emitted in the same grouped write as
sqla=trueandspark=true. - The values stay JSON strings, so
fromJSONyields"3.10", not3.1. Job names (run (3.10)) and thesetup-pythoncondition on'3.13'/'3.14'are unchanged.
- Skipped
changesjob. For Draft and fork pull requests,changesis skipped. The dependent suite jobs are then skipped as before, so the requiredpython-versionsinput is never evaluated with an empty value. - Callers.
test-suite.yamlhas three callers, all intest.yaml, and all three pass the input. No other workflow calls it (benchmarks.yamlanddatabase-sweep.yamldo not). - Branch protection. It has no required status contexts, so fewer matrix jobs on pull requests do not leave an expected check pending.
- actionlint (with ShellCheck on the
runscript) passes. SC2129 was fixed before this review.
No findings.
There was a problem hiding this comment.
Round-one follow-up (a2eb20acf406a0e603ba37b562f1878c562e2c86 → 738d64cf0939592ef4f395eb2a15c379689242de, same base 531185619465b809331a9a8cf2083884365de193): CLEAN
- The repair changes one comment line in
.github/workflows/test.yaml, removing "which the Release workflow checks". - No workflow logic changed: the
PYTHON_VERSIONSvalue, both selection branches, and the outputs are identical.
| | Trigger | PyAthena suite | SQLAlchemy tests | Spark tests | Python versions | | ||
| | --- | --- | --- | --- | --- | | ||
| | Draft pull request | No | No | No | None | | ||
| | Ready pull request from a branch of this repository | Yes | When related files change | When related files change | Newest supported | |
There was a problem hiding this comment.
Self-review round two (claims, callers, operations): FINDINGS, PR description corrected (no code change)
Base 531185619465b809331a9a8cf2083884365de193, head a2eb20acf406a0e603ba37b562f1878c562e2c86.
Findings:
- Claim: "Since Run AWS test suites only on ready pull requests and related changes #837, most Ready pull-request runs on 9/25–9/27 still ran 15 AWS jobs" was wrong. Run AWS test suites only on ready pull requests and related changes #837 merged at 2026-09-26 07:00 UTC, and the 9/25 runs predate it. Measured over the 30 Test runs after the merge:
- 22 ran no AWS jobs.
- 4 ran 5 AWS jobs.
- 4 ran 15 AWS jobs.
The description now states these counts.
- Unstated trade-off.
uv.lockresolves pandas 2.3.3 and numpy 2.2.6 only for Python 3.10, while 3.11+ get pandas 3.0.6 (and numpy 2.4.6/2.5.3). Pull requests on 3.14 therefore never exercise pandas 2 (thepandas>=1.3.0range), and pandas 2-only regressions move to the weekly, dispatch, or pre-release run. This is now stated in WHY. The decision itself (newest only) is Run pull-request AWS tests on the newest Python version only #850's and is not changed here. - Claim: "cuts pull-request AWS cost by about 80%" is an estimate. It is reworded as one: it assumes each version issues about the same queries, and the actual effect is measured in Measure the CI cost after the #834 changes #846.
- Claim: "$20–35 on active days": Cost Explorer shows about $19–37 per active day, excluding the one-off EC2 benchmark on 9/25. Corrected.
Checked and held:
- The docs table and the dispatch sentence match the workflow: PR →
[last], schedule and dispatch → full list. - The comment "keep in sync with the pyproject.toml classifiers, which the Release workflow checks" depends on Require a full Test run on the release commit before publishing #862 merging first, as stated in the PR.
Not observed: a post-merge dispatch or schedule run with 15 jobs (stated in TEST).
There was a problem hiding this comment.
Round-two follow-up (738d64cf0939592ef4f395eb2a15c379689242de): CLEAN, and the description was updated.
- The maintainer decided to merge this PR before Require a full Test run on the release commit before publishing #862. Require a full Test run on the release commit before publishing #862 is being reworked so the Release workflow runs the full matrix itself.
- So the comment no longer claims a release check that is not on master, which resolves the independent reviewer's P2 at its root.
- In the description:
- The merge-order paragraph now matches the decision.
- The pandas 2 trade-off notes that 4.0.0 supports pandas 3.x only (Raise the pandas and pyarrow minimum versions to tested versions #852 comment) and drops Python 3.10 (Drop Python 3.10 support in 4.0.0 #864).
| } >> "$GITHUB_OUTPUT" | ||
| exit 0 | ||
| fi | ||
| echo "python-versions=$(jq -c '[last]' <<< "$PYTHON_VERSIONS")" >> "$GITHUB_OUTPUT" |
There was a problem hiding this comment.
Independent review (relayed): Codex CLI 0.157.0, model gpt-6-sol, reasoning effort high, codex exec -s read-only, session 01a0e0df-6730-7ba1-b3ab-ee60847bd7b4.
- Snapshot: detached worktree at head
a2eb20acf406a0e603ba37b562f1878c562e2c86, base531185619465b809331a9a8cf2083884365de193. The prompt contained the literal diff and repository context only. The review was static: no tests, no network, no edits. The snapshot was unchanged afterwards.
Reviewer result, verbatim:
Surfaces covered: the specified diff, both Test workflows, the release workflow, Python classifiers and tox configuration, and the testing docs. This was a read-only review; I did not run tests.
Verdict: FINDINGS
P1 — .github/workflows/test.yaml:96:
jq '[last]'wraps the last input document. Here the input is one array, so it produces a nested array rather than["3.14"]. A ready PR therefore passes an array as its sole Python matrix value; the Python setup steps cannot use it as a version, so the PR’s AWS suites fail before testing. Use[.[-1]]to select the final array element.P2 — .github/workflows/test.yaml:84: The comment says the Release workflow checks that this list matches the classifiers, but release.yaml has no such check. If a maintainer adds a supported version to
pyproject.tomlbut misses this list, dispatch runs omit that version and the release can still publish. Add the check or remove the claim.Non-blocking suggestion: Add an offline check of the generated PR and dispatch version arrays; this change has no test covering that output.
Author verification:
jq '[last]'nests the array (P1): rejected, disproved by execution.lastwithout arguments is jq's builtindef last: .[-1];(jq ≥ 1.5). It is notlast(f)over input documents.echo '["3.10", "3.11", "3.12", "3.13", "3.14"]' | jq -c '[last]'prints["3.14"](jq 1.8.2 locally; the ubuntu-24.04 runner's apt jq 1.7.1 has the same builtin).- This PR's own Ready CI will show
run (3.14)jobs.
- "which the Release workflow checks" (P2): holds once Require a full Test run on the release commit before publishing #862 is merged, which is the stated merge order.
- The reviewer's snapshot is
origin/master+ this PR, and does not contain Require a full Test run on the release commit before publishing #862. - In Require a full Test run on the release commit before publishing #862,
scripts/verify_release_tests.pyrequires<suite> / run (<version>)to succeed for every classifier version. If a version is added to the classifiers but not toPYTHON_VERSIONS, dispatch runs lack its jobs and the release is refused. - This PR will not be marked Ready before Require a full Test run on the release commit before publishing #862 is merged.
- The reviewer's snapshot is
- Offline test of the version arrays (non-blocking): declined. The selection is two
jqexpressions checked by execution above. The PR's own CI (pull_request) and the next dispatch or schedule run exercise both branches.
There was a problem hiding this comment.
Independent follow-up (relayed): Codex CLI 0.157.0, model gpt-6-sol, reasoning effort high, codex exec -s read-only, session 01a0e21f-5ebf-76d0-bada-7de1f44f0cca.
- Scope:
git range-difffroma2eb20ato738d64cf0939592ef4f395eb2a15c379689242de(base531185619465b809331a9a8cf2083884365de193), plus the full current diff. - The review was static, and the snapshot was unchanged afterwards.
Reviewer result, verbatim:
Covered surfaces: repair delta and full base-to-head diff; all reusable-workflow callers; event gates, suite filters, JSON matrix propagation; Python classifiers, tox configuration, Release workflow, and changed documentation.
Verdict: CLEAN. The repair correctly removes the unsupported Release-verification claim. Eligible PRs select Python 3.14; schedule and dispatch select 3.10–3.14, matching the classifiers. All three callers supply the required input. No unsupported claims or workflow defects found in scope.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
WHAT
test.yaml, thechangesjob now also outputspython-versions:pull_request: the newest supported version only (["3.14"]).scheduleandworkflow_dispatch: every supported version (["3.10","3.11","3.12","3.13","3.14"]).changesjob'sPYTHON_VERSIONS, with a comment to keep it in sync with thepyproject.tomlclassifiers. The newest version is its last element.test-suite.yamltakes a requiredpython-versionsinput (a JSON array). Its matrix is nowfromJSON(inputs.python-versions)instead of a fixed list. All three suites (test,test-sqla,test-sqla-async) pass the selected list.<suite> / run (<version>).docs/testing.md:WHY
Closes #850, part of #834 (the test account's bill for September is heading past $200).
pyproject.toml, oruv.lock.Trade-off:
uv.lockresolves differently by Python version. Pull requests test only the Python 3.14 set. The other sets are tested by the weekly run, a dispatch, or the pre-release run. Notably, pandas 2.3.3 (numpy 2.2.6) is locked only for Python 3.10, while 3.11+ get pandas 3.0.6. So a pandas 2-only regression in the pandas cursor is also found later. This stops mattering in 4.0.0, which supports pandas 3.x only (Raise the pandas and pyarrow minimum versions to tested versions #852) and drops Python 3.10 (Drop Python 3.10 support in 4.0.0 #864).gh workflow run test.yaml --ref <branch>), as before.Merge order: this goes in before #862, to cut the test volume first. #862 will make the Release workflow run every suite on every Python version before publishing, so the reduced pull-request matrix cannot reach a release untested.
TEST
Tested commits: a2eb20a, and 738d64c (comment-only change: the versions comment no longer refers to the release gate).
just scriptspassed. This includes actionlint with ShellCheck on the embeddedrunscript; SC2129 was fixed by grouping the outputs.just docs lintpassed.jqexpressions:pull_request→["3.14"], other events →["3.10","3.11","3.12","3.13","3.14"]. The versions stay strings, so3.10is not turned into3.1.test / run (3.14),test-sqla / run (3.14), andtest-sqla-async / run (3.14).Not yet observed: that a
scheduleorworkflow_dispatchrun after merge still runs all 15 jobs. The next dispatch before a release, or the next weekly run, will show it, and after #862 the Release workflow itself runs the full matrix.🤖 Generated with Claude Code