Cut CI to ~half the job-minutes at equal coverage - #783
Open
wbarnha wants to merge 5 commits into
Open
Conversation
The workflow had grown to 26 jobs and ~49 job-minutes per pull request, most of it spent re-doing work rather than testing anything new. Same interpreters, same drivers, same suites; less of the overhead around them. Run the suite in parallel. `scripts/tests` now passes `-n auto`, sharding across one worker per core. Measured on a 4-core runner with coverage on, that is 127s -> 49s, and pytest-cov merges the per-worker data files, so the report is byte-identical: 11790 statements, 473 missed, 2562 branches, 99 partial, 95% total, before and after. One test blocked this -- test_wrappers built its parametrize IDs from `datetime.utcnow()` at import time, so every xdist worker collected a different ID and the run aborted. It is now a fixed instant, which nothing in the test depended on. Install with uv. A composite action provisions the interpreter, uv and the dependencies for every job. Installing requirements/test.txt goes from 40.6s to 4.8s, and the eight hand-copied setup blocks -- which is how the cache key and the requirements list had already drifted apart between jobs -- collapse to one place. Fold the confluent legs into the matrix. Six dedicated jobs each paid a full checkout and install to run one file for six seconds. Installing the ckafka extra on every leg costs ~2s and runs the same tests, which stop being `importorskip`-ed: 2494 -> 2555 tests collected. Fold the redis job into the Kafka integration job as a service container, and run all of tests/integration there; each suite still skips itself when its service is unreachable, so neither can pass by not running. Move PyPy to master pushes. It was the single most expensive job (~7.5 minutes, ~15% of a pull request) and it is advisory -- `continue-on-error`, and deliberately not in `check`'s `needs`. master only advances through this workflow, so coverage per merged commit is unchanged. Stop rebuilding docs, CodeQL databases and benchmarks for branches that cannot affect them, and group dependabot into one weekly pull request per ecosystem instead of a daily stream of one-line bumps, each of which was paying for the entire matrix. Security advisories are unaffected. Net: 26 jobs / ~49 job-min -> 19 jobs / ~23 job-min per pull request, and the main workflow drops from 359 to 249 lines of configuration. `check` keeps its name and its `needs`, so branch protection is untouched. actionlint reports the same three pre-existing findings as before. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VnDyaVG75qRRvT3gKGcgbT
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 75aab992ff
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+25
to
+29
| paths: | ||
| - docs/** | ||
| - faust/** | ||
| - requirements/docs.txt | ||
| - requirements/requirements.txt |
There was a problem hiding this comment.
Add setup.py and pyproject.toml to both Pages path lists. The build explicitly installs the checkout with uv pip install ., and docs/conf.py derives the rendered documentation version from the installed distribution, so a PR that only changes package discovery, build metadata, or setuptools-scm configuration can change or break the documentation while this workflow is skipped.
Useful? React with 👍 / 👎.
Comment on lines
+12
to
+16
| - faust/** | ||
| - tests/bench/** | ||
| - extra/tools/ci_benchmark.py | ||
| - requirements/requirements.txt | ||
| - .github/workflows/performance-benchmark.yml |
There was a problem hiding this comment.
Add setup.py and pyproject.toml to this filter. extra/tools/ci_benchmark.py installs the current checkout through the project's build configuration before timing it, so a packaging-only PR that changes package contents, dependencies, or the NO_CYTHON build behavior can change which implementation is benchmarked—or make installation fail—without running this PR check.
Useful? React with 👍 / 👎.
astral-sh/setup-uv stopped publishing floating major aliases after v7, so `@v10` is not a ref at all and every job using the composite action died at "Unable to resolve action `astral-sh/setup-uv@v10`" before running a step. Only the exact tags (v8.0.0, v9.0.0, v10.0.0) exist past v7. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VnDyaVG75qRRvT3gKGcgbT
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #783 +/- ##
=======================================
Coverage 96.20% 96.20%
=======================================
Files 110 110
Lines 11790 11790
Branches 1281 1281
=======================================
Hits 11343 11343
Misses 350 350
Partials 97 97 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Two failures from the first green-field run of the new pipeline. uv was picking the wrong interpreter. `UV_SYSTEM_PYTHON=1` alone tells uv to install outside a virtualenv, but leaves it to *find* the interpreter on PATH -- and its discovery skips free-threaded builds unless the request names a `t` ABI. On the 3.13t leg it walked past the interpreter setup-python had just installed and settled on Debian's /usr/bin/python3, which failed as "externally managed" and would have been the wrong interpreter even if it had succeeded. Pass setup-python's `python-path` output as UV_PYTHON so the target is named rather than guessed, on every leg including pypy, and export it to the job env for later steps that shell out to uv (the docs build). confluent-kafka has no cp315 wheel, so putting the ckafka extra on every leg made the 3.15 legs fall back to its sdist, which needs librdkafka headers the runner does not have -- "fatal error: librdkafka/rdkafka.h: No such file or directory" took down the whole install before any test ran. Install the extra everywhere except 3.15. Everything else in that run was green: lint, both integration legs, all ten 3.10-3.14 legs, and the 3.14t free-threaded leg end to end. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VnDyaVG75qRRvT3gKGcgbT
Sharding the suite with `-n auto` is a large win on 3.10-3.13 and a large loss after it. Measured on the matrix: 65-88s on 3.10-3.13 against ~130s serial, but 380s on 3.15 and past 440s on 3.14, where serial is ~150s. The 3.14 legs are not advisory and carry a 10-minute timeout, so that was heading for a red required check rather than merely a slow one. Why the newer interpreters behave this way is not understood; rather than guess at a cause, restrict sharding to the versions where it is measured to help and leave a note to re-measure. `scripts/tests` already puts "$@" after its own `-n auto`, so a trailing `-n0` is enough to put a leg back on one process. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VnDyaVG75qRRvT3gKGcgbT
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
3 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.
The workflow had grown to 26 jobs and ~49 job-minutes per pull request,
most of it spent re-doing work rather than testing anything new. Same
interpreters, same drivers, same suites; less of the overhead around them.
Run the suite in parallel.
scripts/testsnow passes-n auto, shardingacross one worker per core. Measured on a 4-core runner with coverage on,
that is 127s -> 49s, and pytest-cov merges the per-worker data files, so the
report is byte-identical: 11790 statements, 473 missed, 2562 branches, 99
partial, 95% total, before and after. One test blocked this -- test_wrappers
built its parametrize IDs from
datetime.utcnow()at import time, so everyxdist worker collected a different ID and the run aborted. It is now a fixed
instant, which nothing in the test depended on.
Install with uv. A composite action provisions the interpreter, uv and the
dependencies for every job. Installing requirements/test.txt goes from 40.6s
to 4.8s, and the eight hand-copied setup blocks -- which is how the cache key
and the requirements list had already drifted apart between jobs -- collapse
to one place.
Fold the confluent legs into the matrix. Six dedicated jobs each paid a full
checkout and install to run one file for six seconds. Installing the ckafka
extra on every leg costs ~2s and runs the same tests, which stop being
importorskip-ed: 2494 -> 2555 tests collected.Fold the redis job into the Kafka integration job as a service container, and
run all of tests/integration there; each suite still skips itself when its
service is unreachable, so neither can pass by not running.
Move PyPy to master pushes. It was the single most expensive job (~7.5
minutes, ~15% of a pull request) and it is advisory --
continue-on-error,and deliberately not in
check'sneeds. master only advances through thisworkflow, so coverage per merged commit is unchanged.
Stop rebuilding docs, CodeQL databases and benchmarks for branches that
cannot affect them, and group dependabot into one weekly pull request per
ecosystem instead of a daily stream of one-line bumps, each of which was
paying for the entire matrix. Security advisories are unaffected.
Net: 26 jobs / ~49 job-min -> 19 jobs / ~23 job-min per pull request, and the
main workflow drops from 359 to 249 lines of configuration.
checkkeepsits name and its
needs, so branch protection is untouched. actionlintreports the same three pre-existing findings as before.
Co-Authored-By: Claude Opus 5 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01VnDyaVG75qRRvT3gKGcgbT