Open omus opened 6 months ago
It appears to me that the cache-name and include-matrix inputs were designed with only self-testing this package
For the record, not true. They were tested on a large complicated private CI workflow and public OSS packages with more standard CI.
Re-evaluate if we need to include the github.run_attempt as part of the key
If you don't the cache won't be saved in re-runs. The goal of the recent redesign was to basically make the depot get restored>saved>restored>saved... through subsequent runs, to support how a non-ephemeral julia depot works. Not including the run_attempt will prevent it being re-saved.
Allow users to specify the entire cache-key. When a user specifies the cache-key the include-matrix input should be disabled by default.
If we keep cache-name=julia-cache-{job-run}
and include-matrix=true
as defaults, and add a cache-key
that if set ignores both and sets the whole key, then that could work. We'd probably want to include a note on that field about the benefit of including the run and attempt ids through.
If you don't the cache won't be saved in re-runs.
Aren't we just saving redundant information though? What can change between re-runs such that we need to save a new cache information? I'm quite possibly overlooking something with the design of julia-actions/cache
.
add a
cache-key
that if set ignores both and sets the whole key, then that could work.
I think that would be a good non-breaking approach. Ideally I think we'd rename cache-name
to cache-prefix
though. We may also want to add an output of cache-key
from this action. That may be a good approach for the tests to generate a key internally and using cache-key
when performing the restore portion of the test.
Aren't we just saving redundant information though? What can change between re-runs such that we need to save a new cache information? I'm quite possibly overlooking something with the design of julia-actions/cache.
The *_usage.toml
files will get updated usage times which helps the auto Pkg.gc()
do sensible things.
rename cache-name to cache-prefix
Sounds good to queue up for the next breaking change. We can make an issue for that.
Outputting cache-key
for the self-tests here sounds good.
When a user wants to specify a cache name they typically want to also use
include-matrix: false
. It appears to me that thecache-name
andinclude-matrix
inputs were designed with only self-testing this package. I think from a users perspective the following makes more sense:cache-key
with the expectation that users will ensure the key is unique for concurrent jobs.cache-key
theinclude-matrix
input should be disabled by default.Additionally, there are some other aspects around our generated cache key that should change:
github.run_attempt
as part of the key. I believe that GitHub action runs attempts are not allowed to happen concurrently, should generate the same manifest, and we only needgithub.run_id
to deal with cache immutability: https://github.com/actions/cache/blob/main/tips-and-workarounds.md#update-a-cache