Skip to content
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

Add inputs.cache-save-always #34

Merged
merged 9 commits into from
Jan 23, 2024
Merged

Add inputs.cache-save-always #34

merged 9 commits into from
Jan 23, 2024

Conversation

stackptr
Copy link
Member

Adds an input that borrows the save-always input from v4 of actions/cache, which will run the post step to save the cache even if a failure occurs. Using the added cache-save-always input saves the cache when a build fails, which will speed up subsequent runs which restore the partial build. This comes at the cost of requiring a build when there is an exact key match, since the cache from a failed build will have the same key as a completed build.

Adds an input that borrows the `save-always` input from [v4 of `actions/cache`][0], which will run the post step to save the cache even if a failure occurs. Using the added `cache-save-always` input saves the cache when a build fails, which will speed up subsequent runs which restore the partial build. This comes at the cost of requiring a build when there is an exact `key` match, since the cache from a failed build will have the same `key` as a completed build.

[0]: https://github.com/actions/cache/tree/main#v4
@stackptr stackptr self-assigned this Jan 18, 2024
@stackptr
Copy link
Member Author

This will require a major version since this changes the noop-behavior on a full cache hit. If the trade-offs seem worthwhile (and I'm not missing anything about how Stack might treat these artifacts) then I'll bump the version and update the docs.

Copy link
Member

@pbrisbin pbrisbin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Love that this is opt-in. I think that makes it fine that there's a trade-off ("build even on full hit"). I don't follow why this would be a major version bump. It seems to be adding a new option with the same behavior by default... e.g. minor.

action.yml Outdated Show resolved Hide resolved
@stackptr
Copy link
Member Author

I don't follow why this would be a major version bump. It seems to be adding a new option with the same behavior by default... e.g. minor.

Sounds good; I wasn't sure how to characterize the risk of removing the noop. The behavior is certainly the same and what is changed in the default scenario, not making use of the input, is an implementation detail.

action.yml Outdated Show resolved Hide resolved
Saves the set of build artifacts after a build failure with a `-partial` suffix on the cache key. A full cache-hit will therefore only occur when restoring the cache saved after a successful build, allowing for the subsequent `stack build` and cache-save steps to be skipped.
Status check functions can only be used in `if` conditionals, so the use of a cache key suffix needs to be determined in a conditionally-run step.
Checks that the workflow has not been cancelled (i.e., is succeeding or failed) and that `cache-save-always: true` when building and saving the cache. This extends the existing conditional on the `cache-hit` outputs, making the previously-implied `success()` check explicit.

When `cache-save-always: false`, the default behavior is unchanged for the action: steps either no-op if a cache-hit occurred, or build and save otherwise.

When `cache-save-always: true`, a building and saving the cache occurs on success or failure.
action.yml Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@stackptr stackptr merged commit 178359c into main Jan 23, 2024
27 checks passed
@stackptr stackptr deleted the corey/save-always branch January 23, 2024 19:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants