-
Notifications
You must be signed in to change notification settings - Fork 27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ENH: Fix caching-related warnings in GHA build-test-publish
CI
#475
base: master
Are you sure you want to change the base?
ENH: Fix caching-related warnings in GHA build-test-publish
CI
#475
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #475 +/- ##
=======================================
Coverage 83.85% 83.85%
=======================================
Files 30 30
Lines 2819 2819
Branches 365 365
=======================================
Hits 2364 2364
Misses 384 384
Partials 71 71 ☔ View full report in Codecov by Sentry. |
e6a245b
to
04618e8
Compare
build-test-publish
CI
Warnings are gone: @effigies The I tried the problem had been solved naming the AFNI cache keys with distinct names:: But it does not seem to be the case: Any clue? |
Fix caching-related warnings in GHA `build-test-publish` CI: - Save the `apt-get` cache in a directory other than `/var/lib/apt` to avoid permission issues. - Make the AFNI cache key name be specific to the CI matrix configuration to avoid clashes across cache names. Use the root part as the restore key name so that any cache found can be restored, as the AFNI version being installed is the same across configurations. - Check if the AFNI cache exists before trying to install it in GitHub Actions `build-test-publish` CI workflow. Fixes: ``` Failed to save: "/usr/bin/tar" failed with error: The process '/usr/bin/tar' failed with exit code 2 ``` The full log showing ``` 2024-12-19T13:35:45.2830631Z [command]/usr/bin/tar --posix -cf cache.tzst --exclude cache.tzst -P -C /home/runner/work/sdcflows/sdcflows --files-from manifest.txt --use-compress-program zstdmt 2024-12-19T13:35:46.5536670Z Failed to save: Unable to reserve cache with key afni-v1, another job may be creating this cache. More details: Cache already exists. Scope: refs/heads/master, Key: afni-v1, Version: d04022ae09f8f21b8c0f9f00e4a784b6e510fe6a47d30aa3b0853a42885b92cb 2024-12-19T13:35:46.5924639Z Post job cleanup. 2024-12-19T13:35:46.7348688Z [command]/usr/bin/tar --posix -cf cache.tzst --exclude cache.tzst -P -C /home/runner/work/sdcflows/sdcflows --files-from manifest.txt --use-compress-program zstdmt 2024-12-19T13:35:46.8326360Z /usr/bin/tar: ../../../../../var/lib/apt/lists/lock: Cannot open: Permission denied 2024-12-19T13:35:47.1807249Z /usr/bin/tar: ../../../../../var/lib/apt/lists/partial: Cannot open: Permission denied 2024-12-19T13:35:47.2842971Z /usr/bin/tar: Exiting with failure status due to previous errors 2024-12-19T13:35:47.2851756Z ##[warning]Failed to save: "/usr/bin/tar" failed with error: The process '/usr/bin/tar' failed with exit code 2 ``` raised for exmaple in: https://github.com/nipreps/sdcflows/actions/runs/12413644206
04618e8
to
7cc0732
Compare
- name: Restore cache for AFNI | ||
id: cache-afni | ||
uses: actions/cache@v4 | ||
with: | ||
path: /opt/afni | ||
key: afni-v1 | ||
key: afni-v1-${{ matrix.python-version }}-${{ matrix.dependencies }}-${{ matrix.marks }} | ||
restore-keys: | | ||
afni-v1 | ||
afni-v1- | ||
- name: Install AFNI | ||
if: steps.cache-afni.outputs.cache-hit != 'true' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We set up this cache to not depend on the specific job, as its contents should not vary. Not saving multiple caches is a feature, not a bug.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My memory is vague now, but looking at the commit message maybe GHA was showing some warning about name clashes (e.g. if two processes were trying to write to the same file)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When there's a cache miss, each job will populate the directory, and the first to finish will successfully save the cache. That's fine and expected. The alternative would be to create a separate job to ensure the cache exists, and insert it into the workflow ahead of these.
@@ -88,7 +88,7 @@ jobs: | |||
- uses: actions/checkout@v4 | |||
- uses: actions/cache@v4 | |||
with: | |||
path: /var/lib/apt | |||
path: ${{ runner.temp }}/cache-linux |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Warning: Path Validation Error: Path(s) specified in the action for caching do(es) not exist, hence no cache is being saved.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should just drop the APT cache, since it hasn't worked, and it would only save us up to 15s in 8m jobs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fine if we want to drop this; I do not have a strong opinion. Let me know if you firmly believe we should go ahead and remove it, or whether you want other maintainers to weigh in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's clearly not serving its purpose, so it's safe to remove. If you or @oesteban want to take another stab at reducing this time, you're welcome to, but if you're just motivated to reduce warnings, the simple path forward is remove the cache.
Fix caching-related warnings in GHA
build-test-publish
CI:apt-get
cache in a directory other than/var/lib/apt
to avoid permission issues.build-test-publish
CI workflow.Fixes:
The full log showing
raised for exmaple in:
https://github.com/nipreps/sdcflows/actions/runs/12413644206