Bring the CI cache back under budget - #794
Merged
Merged
Conversation
The Actions cache sits at 14.04 GB against a 10 GB limit, so eviction runs constantly and the test-data caches are what it takes. Every eviction re-downloads the whole registry from raw.githubusercontent.com one file at a time with no retry, and a single 30 s read timeout fails the job before any test runs. Of the 41 live caches, 35 are environment-* holding 10.37 GB. The cache-environment-key embedded $(date +%Y-%m-%d), so 3 platforms x 4 python versions x 2 env files minted ~35 fresh caches every day. The date is not pointless - with unpinned deps it is what picks up upstream updates - so it becomes an ISO week rather than going away. %G, not %Y: ISO week numbers pair with the ISO year and %Y-W%V collides across a new year. That this was eviction and not a key-design problem: at registry hash 6f304af6 the Windows and macOS data caches survived while the Linux one was gone and had to be re-primed. Same key, same day, one OS evicted and two kept. Also: - The data cache gains a restore-keys fallback, so a registry change reuses the previous cache and pooch fetches only the added files. The key now puts cache_number before the registry hash so it can sit in the prefix; otherwise bumping it - the documented reset - would fall back onto the cache the bump meant to discard. - Priming prunes files no longer in the registry, which a prefix restore would otherwise carry into every later cache. It compares against the full registry, not the primed subset, so a legitimately fetched large file stays. - pooch gets retry_if_failed=3, and its floor moves to >=1.3, where that option was added; on 1.2 it would have raised TypeError. - fail-fast: false on the test and min-deps matrices. A cancelled job never reaches its cache-save step, so one unrelated failure both hid the other results and made every other job re-prime next run. - The test matrix drops 3.11 (each entry costs one environment cache per OS) and min-deps moves from 3.13/3.14 to 3.11/3.14 - same job count, but it now covers the oldest supported version, which is the more useful thing for a minimum-dependency run and keeps 3.11 verified. - requires-python moves to >=3.11 and the 3.10 classifier goes. No CI matrix ever covered 3.10, so the package claimed support it did not verify, and 3.10 is EOL in October 2026.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughChangesPython support and CI cache behavior
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #794 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 164 164
Lines 17713 17696 -17
=========================================
- Hits 17713 17696 -17
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
micromamba resolved python=3.14 to the free-threaded (cp314t) build on linux-64 once the environment cache key changed and forced a fresh solve; scipy is what tips the solver there. That job then had no orjson wheel (pymseed dependency) and failed to build it from source. python-gil keeps the matrix on GIL-enabled builds; free-threading has its own workflow. Also aligns the pooch floors in the env files with pyproject.
The test-data cache key is a GitHub Actions detail, so keep it in the CI script that consumes it rather than in dascore.utils.downloader. Drops TestDataCacheInfo, get_test_data_cache_info, and their tests.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/scripts/export_test_data_cache_env.py (1)
18-35: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd focused tests for the new cache-key contract.
These helpers now control both
actions/cache/restoreandactions/cache/save, while the previous library-side cache-key tests were removed. Add cases coveringDATA_VERSION/cache-number isolation, registry-hash suffixes, and exact restore-prefix formatting.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/scripts/export_test_data_cache_env.py around lines 18 - 35, Add focused tests for get_restore_prefix and get_key covering DATA_VERSION and cache_number isolation, registry_hash appended as the key suffix, and the exact restore-prefix formatting including the trailing separator. Replace reliance on removed library-side cache-key coverage while validating both helpers’ cache-key contract.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.github/scripts/export_test_data_cache_env.py:
- Around line 18-35: Add focused tests for get_restore_prefix and get_key
covering DATA_VERSION and cache_number isolation, registry_hash appended as the
key suffix, and the exact restore-prefix formatting including the trailing
separator. Replace reliance on removed library-side cache-key coverage while
validating both helpers’ cache-key contract.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 0451b32e-bfea-4e5a-92ea-e65e5d1aca3b
📒 Files selected for processing (6)
.github/actions/mamba-install-dascore/action.yml.github/doc_environment.yml.github/scripts/export_test_data_cache_env.pydascore/utils/downloader.pyenvironment.ymltests/test_utils/test_downloader.py
💤 Files with no reviewable changes (1)
- tests/test_utils/test_downloader.py
🚧 Files skipped from review as they are similar to previous changes (1)
- dascore/utils/downloader.py
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
1 participant
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.
Description
The repo's GitHub Actions cache sits at 14.04 GB against a 10 GB limit, so LRU eviction runs continuously and the test-data caches are the collateral. Every eviction forces a re-download of the whole registry from
raw.githubusercontent.com, one file at a time with no retry, which is where the intermittent CI failures come from — a single 30 s read timeout fails the whole job before any test runs.Measured breakdown of the 41 live caches:
environment-*data-*The environment caches are 74% of the budget on their own.
Why the environment caches explode
cache-environment-keyembedded$(date '+%Y-%m-%d'), minting a brand-new key every day.setup-micromambathen appends platform, python-args, and an env-file content hash, so the real multiplier is 3 platforms × 4 python versions × 2 env files ≈ 35 fresh caches per day at 240–560 MB each.The date isn't pointless — with unpinned deps it's what picks up upstream package updates — so it's now ISO week (
%G-W%V) rather than deleted. Same intent, ~7× less churn.%Grather than%Ybecause ISO week numbers pair with the ISO year and%Y-W%Vcollides across a new-year boundary.Evidence this was eviction rather than a key-design problem: at registry hash
6f304af6the Windows and macOS data caches survived while the Linux one was gone, so a run had to re-prime and re-save it. Same key, same day, one OS evicted and two kept is the LRU signature. The data cache key itself —data-{OS}-{DATA_VERSION}-{sha256(registry)}-{bust}— is content-addressed and correct, and is unchanged here.Changes
Weekly environment cache key instead of daily (above).
restore-keysfallback for the data cache. A registry change previously missed the exact key and re-downloaded all 47 files. It now falls back todata-{OS}-{DATA_VERSION}-{cache_number}-, and pooch fetches only the added files. A prefix-only match reportscache-hit != 'true', so the prime and save steps still run and publish under the new exact key.get_keybuilds onget_restore_prefixso the two cannot drift.The key is reordered to put
cache_numberbefore the registry hash, so it can live in the prefix. Without that, bumpingcache_number— the documented manual reset — would fall back onto the very cache the bump was meant to discard, silently doing nothing. The reorder costs one re-prime per OS on first run; a working reset knob is worth it.The cache key now lives in the CI script, not the library.
TestDataCacheInfo/get_test_data_cache_infowere removed fromdascore.utils.downloaderand the key/prefix construction moved into.github/scripts/export_test_data_cache_env.py, its only consumer. A GitHub Actions cache key is not library API, and its tests do not belong in the test suite; DASCore stays agnostic about the CI backend.retry_if_failedstays — retrying a transient read timeout is useful to anyone fetching the registry, not just CI.Unregistered files are pruned before saving. Restoring an older cache carries along files since dropped from the registry, which would otherwise ride into every later cache forever. Priming now deletes anything not in the full registry — the full one, not the primed subset, so a large file that was legitimately fetched is kept.
retry_if_failed=3on the pooch fetcher, with thepoochfloor raised>=1.2→>=1.3(the option landed in 1.3, so on 1.2 this would have been aTypeErrorat import). Priming pulls the whole registry in one pass, so one transient read timeout used to fail the job.fail-fast: falseon thetest_codeand min-deps matrices. This is a caching fix as much as a reporting one: a cancelled job never reaches its cache-save step, so one unrelated failure both hid the other results and forced every other job to re-prime on the next run. (network_testsalready set this.)Python matrix trimmed from
3.11–3.14to3.12–3.14. Each entry costs one environment cache per OS.Min-deps moved from
3.13/3.14to3.11/3.14. Same job count and same cache count, but it now covers the oldest supported version, which is the more useful thing for a minimum-dependency run — and it keeps 3.11 verified despite leaving the full matrix.requires-pythonraised to>=3.11and the 3.10 classifier dropped. 3.10 was never in any CI matrix, so the package claimed support it did not verify; it also reaches end of life in October 2026. Every versionpyprojectnow claims is tested somewhere.python-giladded to the micromambacreate-args. This is the fallout of the key change, and worth spelling out:python=3.14alone lets the solver pick the free-threadedcp314tbuild on linux-64 (scipyis what tips it —python=3.14 scipy>=1.15.0reproduces it locally,python=3.14alone does not).devonly passed because its ubuntu 3.14 job kept restoring a pre-cp314tenvironment cache; the first fresh solve — this PR's, ordev's own next week — hits it. The job then has noorjsonwheel (apymseeddependency), tries to build it from source, andorjsonrefuses to build against free-threaded CPython.python-gilpins the GIL-enabled build; it resolves cleanly for 3.11–3.14, and free-threading keeps its own coverage intest_free_threaded.yml.Expected steady state: ~5 environment caches per week plus data, comfortably inside the budget. A further ~2.3 GB returns on its own when
devreachesmaster, since dev'sdownloader.pyrework already cut the data cache from 1014 MB to 237 MB per OS.Not included
Pruning the now-unreachable dated
environment-*caches will speed this up, but is an operational step rather than a repo change.Changelog
Checklist
I have (if applicable):
Summary by CodeRabbit
Breaking Changes
Bug Fixes
Chores