Allow to skip the save post-step

[roni-frantchi]

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

[domjtalbot]

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?

[roni-frantchi]

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.

[joshmgross]

Thanks for your suggestion!

Due to the type issues with env always being a string, I think we'd need this to be an input rather than an environment variable so that we can properly evaluate it.

Also this does not make the cache key option, so could you update your PR description to remove "Fixes #210"?

[roni-frantchi]

Thanks for reviewing @joshmgross !

I think we'd need this to be an input rather than an environment variable so that we can properly evaluate it

post-if is undocumented and despite my best efforts to make it evaluate inputs I couldn't - only env was available to me.

Due to the type issues with env always being a string...

I did address this one though - so perhaps using env would be acceptable now?

[anders-kiaer]

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 🙂

[mikeage]

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?

[zwass]

@joshmgross Can you please re-review/merge this? I'd like to use it and would prefer not to rely on a fork.

[joshmgross]

👋 Hey @zwass, the @actions/artifacts-actions team should be able to help out here.

[dhadka]

TY for proposing this change. Just some minor comments on the README changes, but otherwise looks good. Please ping me when ready and I can 🚢 this.

[dhadka]

Given that this is the default example and skipping cache save is a special case, I wouldn't include this here. Perhaps move this into the Examples.md file and we can add some specific examples of where this is useful.

[dhadka]

Something we can also highlight in the example is how users can either set this on the cache step as shown here or using echo "CACHE_SKIP_SAVE=true" >> $GITHUB_ENV.

[roni-frantchi]

I think the changes you've suggested on the readme file capture it well enough

[rene-leanix]

@roni-frantchi & @dhadka, would be great if this PR could be merged to reduce the workflow runtime when a cache-update isn't required. Seems like only a small adjustment would be needed?
Not sure if this issue could be addressed in this PR too: skipping the post-step altogether when there was a cache-hit.

[roni-frantchi]

Not sure if this issue could be addressed in this PR too

I'm not sure that it can either...
Seeing so many are expecting it, maybe we should explore doing it on another PR?..

Seems like only a small adjustment would be needed?

Sorry @rene-leanix it's been a while - what adjustment would that be?..

[rene-leanix]

@roni-frantchi, I was just referring to @dhadka's comments above in this thread: here and here.

[dhadka]

This should be removed now that it's set on the cache-primes step.

[roni-frantchi]

64daede

[ollydev]

@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.

[zwass]

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).

[jmoguillansky-gpsw]

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?

[iuioiua]

@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.

[Jimimaku]

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.post

Behebt #350 Behebt #334

[kotewar]

Hey all, 👋🏽

We have created a discussion with a proposal that will solve this problem, do let us know your feedback, thanks 😊

[kotewar]

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.