actions
Allow to skip the save post-step
This is similar to #474 - but improves on that by skipping the post (save) step entirely, saving tens of seconds on each run.
Fixes #350 Fixes #334
I really like this approach! It's helped to save more valuable seconds on my builds than other solutions.
What do you think about the variable name being something like READ_ONLY?
Thanks @domjtalbot.
Sure thing.
If we get a positive response from maintainers on pushing it in, I'd be happy to make that adjustment along with any others suggested.
I see this PR has gone through a first review by @joshmgross already. Will it be merged in the near future, or should use cases needing to skip the save step use e.g. a fork of this action?
Regarding using environment variable: I think this is a good choice, if my understanding is correct that previous steps in the job then can dynamically/during runtime instruct the cache action save/post step through https://docs.github.com/en/actions/reference/workflow-commands-for-github-actions#setting-an-environment-variable.
Thanks :slightly_smiling_face:
This looks wonderful; one question, if you don't mind. I have a very large cache (~750 MB), and when I have multiple PRs in progress, it appears that the 5GB cache gets filled up with those, and the cache from main gets pushed out. If I were to use this fork and set the option to skip the saving on the PRs but not on the main branch, this would keep the main cache as a base for the all of the PRs, right?
@joshmgross Can you please re-review/merge this? I'd like to use it and would prefer not to rely on a fork.
👋 Hey @zwass, the @actions/artifacts-actions team should be able to help out here.
@dhadka This is what I wanted #498 for, didn't see this PR though.
Right now cache will never be saved if the job fails. Example: Spend 10mins building dependencies (succesfully) but a future job (such as failing a test) fails. Cache will not save!
Can we add something like:
env:
CACHE_FORCE_SAVE: true
I think it would be something like this:
post-if: success() && env.CACHE_SKIP_SAVE != 'true' || failed() && env.CACHE_FORCE_SAVE == 'true'
Thanks.
Perhaps @ollydev's request could be merged into this one with a single flag?
CACHE_SAVE=always - Always save regardless of errors in preceding steps.
CACHE_SAVE=never - Never save regardless of errors.
CACHE_SAVE=default or Not set - Default behavior (as before this PR).
failed() && env.CACHE_FORCE_SAVE == 'true': how do we know the content to cache was generated successfully? I think we need some mechanism to indicate that generating the cache content was successful, and then cache it immediately, why wait until the end of the workflow?
Please ping me when ready and I can 🚢 this.
@dhadka Looks like all the comments have been addressed, can we get this merged?
@bgianfo There's a discussion happening over in https://github.com/actions/cache/pull/571 on a similar issue. Essentially, there are a few open issues that are somewhat related (better control over if and when the cache is saved). Instead of adding in a bunch of piecemeal fixes, is there a more generalizable solution?
failed() && env.CACHE_FORCE_SAVE == 'true': how do we know the content to cache was generated successfully? I think we need some mechanism to indicate that generating the cache content was successful, and then cache it immediately, why wait until the end of the workflow?
@jmoguillansky-gpsw Yes, this is essentially a limitation of running save as a "post step", which only runs at the end of the workflow. You're essentially looking for a standalone save step that a user can run at any point in their workflow. See https://github.com/actions/cache/pull/571#issuecomment-839123699.
How does this work when you are using multiple cache actions in one workflow job?
I have a cache action for the caching node modules and I have another cache action for a benchmark build results.
If I understand this correctly, when I echo "CACHE_SKIP_SAVE=true" >> $GITHUB_ENV then it would prevent both the cache for the node modules and the cache for the benchmark to not save after the workflow is completed. Is that correct? I really want to target one of the cache actions to not save (ex: don't save the benchmark cache unless the branch is main), not just blindly affect all of the cache saves.
It seems like there should be a better way for specifically preventing a single cache action from saving without all cache actions being prevented from caching?
@dakale @roni-frantchi Seems like this change is of interest to many users. As we are not bringing this PR to conclusion downstream users are turning to use forks. Would probably be a good idea to bring this to conclusion
Thanks!
@dakale I've addressed all of you comments - is there anything other request in this thread that you think must be taken care of in this PR before merging?
I would rather add any improvements later on and get this one going.
Is this going to get merged soon? The ability to skip the save post-step is something that will be very useful for our use case as well.
@roni-frantchi, is it possible for you to update the branch? It'd remove the deprecation warnings around save-state, set-output and Node.js 12 for those using the fork.
Dies ist ähnlich wie # 474 - verbessert dies jedoch, indem der (gespeicherte) Schritt vollständig übersprungen wird, wodurch bei jedem Lauf Dutzende von Sekunden gespart werden.
postBehebt #350 Behebt #334
Hey all, 👋🏽
We have created a discussion with a proposal that will solve this problem, do let us know your feedback, thanks 😊
Hey everyone 👋🏽
Two new actions actions/cache/restore and actions/cache/save are now available with tag v3 to everyone for use. These can now be used to achieve granular control on the restore and save steps in the cache action.
Do try them and let us know your feedback. We hope these new actions will take care of your use cases. 🙇🏽
Closing this PR now as we believe the new actions will take care of the same, feel free to reopen it or create an issue if need be.