Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 48 additions & 32 deletions .github/workflows/test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,10 @@ jobs:
# requests run none. A ready pull request always runs the PyAthena suite; it
# runs the SQLAlchemy tests (the compliance suites and the PyAthena suite's
# SQLAlchemy tests) and the Spark tests only when their code, tests,
# dependencies, or this workflow change. Pull requests and the schedule test
# the newest Python version; a dispatch tests the requested versions or
# every version, and the Release workflow every version.
# dependencies, this workflow, the top-level modules of pyathena and
# pyathena.aio, or the shared test fixtures change. Pull requests and the
# schedule test the newest Python version; a dispatch tests the requested
# versions or every version, and the Release workflow every version.
changes:
if: >-
github.event_name != 'pull_request' ||
Expand All @@ -80,16 +81,53 @@ jobs:
permissions:
pull-requests: read
outputs:
sqla: ${{ steps.filter.outputs.sqla }}
spark: ${{ steps.filter.outputs.spark }}
python-versions: ${{ steps.filter.outputs.python-versions }}
sqla: ${{ github.event_name != 'pull_request' || steps.paths.outputs.sqla == 'true' }}

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Self-review round two, expanded scope (claims and operational effects): CLEAN (PR body rewritten)

  • Scope: claims in the 7439d76 commit message, the rewritten PR body, and the workflow comments, over 8679f04a36f6ffd7190b892c32b1938672ab502f..7439d76ea0e9b78797eca35bb82cff109f39e678.
  • Claims checked:
    • "reads the same pull request files API": getChangedFilesFromApi calls pulls.listFiles (src/main.ts:204-217 at v4.0.3).
    • "selection is unchanged for every tracked file": per-file comparison over git ls-files (278 files) is identical.
    • "a renamed file now also matches by its previous path": src/main.ts:228-238.
    • "only the job's pull-requests: read token": job-level permissions replace the workflow-level ones.
    • "other events still run every suite": the output expressions.
  • Operator: a new third-party action dependency. It is maintained (v4.0.3 published 2026-08-05, last push 2026-09-27), the repository allows all actions, and pinact tracks the SHA pin. The AWS cost effect is unchanged from the grep -E version, because the selection is identical.
  • Evidence limits (stated in the PR body): the AWS run that passed was on 020dc45, the grep -E version. The action itself runs only on a ready pull request, so its first real run is this PR's Ready run; its log and outputs must be checked before merge. A push at 7439d76 while Ready started a run; returning the PR to Draft cancelled it before the changes job finished, so no AWS job ran.
  • Workflow comment above changes:: still accurate, because the behavior it describes did not change.

spark: ${{ github.event_name != 'pull_request' || steps.paths.outputs.spark == 'true' }}
python-versions: ${{ steps.versions.outputs.python-versions }}
steps:
- id: filter
- id: paths
if: github.event_name == 'pull_request'
uses: dorny/paths-filter@ceb8a2b8f2d89434be7ff52d3de7ec3738c5cc9d # v4.0.3

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Self-review round one, expanded scope (paths-filter migration): CLEAN

  • Scope: full pass over git diff 8679f04a36f6ffd7190b892c32b1938672ab502f..7439d76ea0e9b78797eca35bb82cff109f39e678. The migration commit (020dc45b..7439d76e) replaces the gh api + grep -E selection with dorny/paths-filter, so earlier narrow-repair scoping does not apply.
  • Action behavior, read from src/main.ts and src/filter.ts at the pinned SHA (ceb8a2b8, tag v4.0.3):
    • For pull_request with a token (default github.token), it calls pulls.listFiles with pagination, the same API as before, and needs no checkout.
    • A renamed file is split into an added new path and a deleted previous path.
    • Filters are loaded with js-yaml, nested anchor lists are flattened, and each pattern is matched with picomatch (dot: true). With the default some quantifier, the output is 'true' when any changed file matches any pattern.
  • Selection: a local mirror of that logic (js-yaml 4, picomatch 2.3.1) gives the expected result for 48 cases. Against the previous grep -E patterns at 020dc45, it gives identical sqla/spark results for all 278 tracked files. The only difference is a hypothetical new tests/conftest.py, which the previous tests/(pyathena/(aio/)?)?conftest regex matched and the explicit list does not; no such file exists.
  • Outputs: sqla/spark are github.event_name != 'pull_request' || steps.paths.outputs.<name> == 'true'. Schedule, dispatch, and Release (workflow_call inherits the caller's push event) skip the step and evaluate to true. Pull requests use the action's result. Job outputs are strings, so consumers' == 'true' / != 'true' comparisons are unchanged. Draft and fork pull requests still skip the whole job.
  • Permissions: the job's permissions: pull-requests: read replaces the workflow-level id-token: write. So the third-party action gets only a read token and no OIDC, and it is pinned by full SHA (pinact run --check passes).
  • Failure path: an API failure fails the step and therefore the job, so the test jobs do not run. This is the same as the previous gh api under bash -e.
  • Runtime: the action uses node24, as actions/checkout v6.0.2 already does in this repository.
  • Pre-existing, unchanged: the files API lists at most 3,000 files per pull request, the same as gh api before.
  • Checks: just lint, just scripts (actionlint + 108 script tests), and pinact run --check passed.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Independent review, expanded scope (relayed): CLEAN

  • Reviewer: Codex CLI 0.157.1, model gpt-6-sol, reasoning effort high, --sandbox read-only, --ephemeral, session 01a0f4c7-89b2-7371-8671-28eec96b9bbe. Static review only: no workflow or tests run.
  • Scope: full pass over 8679f04a36f6ffd7190b892c32b1938672ab502f..7439d76ea0e9b78797eca35bb82cff109f39e678 on a detached snapshot. Because the sandbox has no network, the package also contained action.yml, src/main.ts, and src/filter.ts of dorny/paths-filter at the pinned commit ceb8a2b8 (v4.0.3), outside the repository tree. The prompt omitted the PR number, description, commit messages, and prior findings. The snapshot's HEAD was unchanged afterwards; the only untracked entry was that reference directory.
  • Covered: the workflow diff, the output consumers in test.yaml/test-suite.yaml, the Release caller, and the pinned action source.
  • Result:
    • The globs and flattened YAML anchors select the stated paths, dotfiles match, and the comments inside filters parse as YAML comments.
    • sqla/spark become "true"/"false" as downstream jobs expect. When the step is skipped for a non-PR event, both are "true".
    • Python version selection is unchanged.
    • The action is SHA-pinned and gets a token limited to pull-request read access, and changed filenames never reach a shell.
    • An API error fails changes, so the dependent AWS jobs do not run.
  • Pre-existing note: paths-ignore at .github/workflows/test.yaml:16-18 means a PR that changes only docs or Markdown starts no run. So "A ready pull request always runs the PyAthena suite" (line 68) applies only to runs that start. Deferred as pre-existing and unchanged: the comment above on: (lines 11-14) already states that docs-only pull requests start no run.

with:
filters: |
shared: &shared
- .github/workflows/test.yaml
- .github/workflows/test-suite.yaml
- justfile
- pyproject.toml
- uv.lock
# The SQLAlchemy and Spark packages load the top-level modules,
# directly or transitively, and their tests use the shared fixtures.
core: &core
- pyathena/*.py
- pyathena/aio/*.py
- tests/__init__.py
- tests/pyathena/__init__.py
- tests/pyathena/conftest.py
- tests/pyathena/tables.py
- tests/pyathena/util.py
- tests/pyathena/aio/__init__.py
- tests/pyathena/aio/conftest.py
- tests/resources/**
sqla:
- *shared
- *core
- setup.cfg
- pyathena/sqlalchemy/**
- pyathena/aio/sqlalchemy/**
- tests/sqlalchemy/**
- tests/pyathena/sqlalchemy/**
- tests/pyathena/aio/sqlalchemy/**
spark:
- *shared
- *core
- pyathena/spark/**
- pyathena/aio/spark/**
- tests/pyathena/spark/**
- tests/pyathena/aio/spark/**
- id: versions
env:
GH_TOKEN: ${{ github.token }}
EVENT_NAME: ${{ github.event_name }}
REPO: ${{ github.repository }}
PR_NUMBER: ${{ github.event.pull_request.number }}
REQUESTED_VERSIONS: ${{ inputs.python-versions }}
# Every supported version, oldest first; keep in sync with the
# pyproject.toml classifiers.
Expand All @@ -113,28 +151,6 @@ jobs:
;;
esac
echo "python-versions=$versions" >> "$GITHUB_OUTPUT"
if [[ "$EVENT_NAME" != "pull_request" ]]; then
{
echo "sqla=true"
echo "spark=true"
} >> "$GITHUB_OUTPUT"
exit 0
fi
files=$(gh api "repos/$REPO/pulls/$PR_NUMBER/files" --paginate --jq '.[].filename')
printf 'Changed files:\n%s\n' "$files"
shared='^(\.github/workflows/test(-suite)?\.yaml|justfile|pyproject\.toml|uv\.lock)$'
sqla="$shared|^pyathena/(aio/)?sqlalchemy/|^tests/sqlalchemy/|^tests/pyathena/(aio/)?sqlalchemy/|^setup\.cfg$"
spark="$shared|^pyathena/(aio/)?spark/|^tests/pyathena/(aio/)?spark/"
if grep -qE "$sqla" <<< "$files"; then
echo "sqla=true" >> "$GITHUB_OUTPUT"
else
echo "sqla=false" >> "$GITHUB_OUTPUT"
fi
if grep -qE "$spark" <<< "$files"; then
echo "spark=true" >> "$GITHUB_OUTPUT"
else
echo "spark=false" >> "$GITHUB_OUTPUT"
fi

# The three suites create their own schemas and tables, so they run in
# parallel; each is still a separate job for "Re-run failed jobs".
Expand Down
Loading