Add new setting save-on-success.#1571
Conversation
* Proposal to implement actions#1570
b38d23e to
40b3745
Compare
|
What is the status of this? This could really improve our workflows! |
Co-authored-by: Kyle Ross <37418852+Rossbro2@users.noreply.github.com>
Co-authored-by: Kyle Ross <37418852+Rossbro2@users.noreply.github.com>
An owner needs to approve the workflows and merge on green :-) |
|
@salmanmkc any chance you could approve, merge & release this? I've tested this on a fork and this is working great! |
|
@salmanmkc Is there's any chance to merge this? |
Hi, this is not my area, I'm not one of the reviewers for cache |
|
I see, could you please help us and ping a person responsible? |
|
@GhadimiR hey, could you please review this PR? |
| main: 'dist/restore/index.js' | ||
| post: 'dist/save/index.js' | ||
| post-if: "success()" | ||
| post-if: "success() && github.event.inputs.save-on-success" |
There was a problem hiding this comment.
This will not work for the same reason that save-always was removed - #1452.
post-if does not have access to the input context, so this will always be false and completely break saving the cache.
To enable this type of workflow, you need to either:
- Enable accessing the input context in
post-if(requiresactions/runnerchanges) - Use two separate actions for restore and save (this should already work)
|
Hi @eirnym, thanks for contributing, as this change won't work due to the limitation pointed out by @joshmgross, we won't be accepting it. Given that this use-case can be addressed with a combination of save/restore actions, I don't see a need for us to introduce new functionality to achieve the same outcome using a new parameter. |
|
Hi, thank you for your comments. Is it correct to say, that the only way is to write a custom action like it's done in Python setup? |
|
@eirnym that might be the best way to accomplish your use-case, if you want to apply complicated logic to your caching, however for precisely what is described above, it sounds like you could accomplish this with and You can see some examples that may help you here and combine that with simple conditions based on branch name or whatever else you'd like to act on. |
|
In 99% of cases of such customization (on my experience) it's enough to This is important to avoid potential cache poisoning from PRs coming outside. |
Proposal to implement #1570, https://github.com/actions/cache/discussions/1598
Description
Allows to control whether new caches can be written in post action on success.
Motivation and Context
In some cases you want to read but not save a new cache. For instance when you run workflows on a tag action, then you probably want to read caches from the main/default branch, but you most likely do not want to create new caches. The current solution is to separate restore and save actions which is cumbersome.
In #1452 the always_save feature was removed.
I propose to add a new config save-on-success that defaults to true and can be set false to prevent cache writing. Now this new value can be set from available context - just like you could when separating the steps - granted the decision must be available upfront. That above described decision is in fact available immediately on job creation. So we can use the new setting to simplify the setup.
How Has This Been Tested?
Local experiments
Screenshots (if appropriate):
Types of changes
Checklist: