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 fail-on-cache-miss option #1036

Merged
merged 14 commits into from
Jan 30, 2023
Merged

Add fail-on-cache-miss option #1036

merged 14 commits into from
Jan 30, 2023

Conversation

cdce8p
Copy link
Contributor

@cdce8p cdce8p commented Dec 22, 2022

Description

Add an advanced option to fail job if cache miss occurs.
Based on the work from @kotewar which was reverted in 11ab7cc.

Motivation and Context

Fixes #955 and partially #1020 (comment)

This is already possible via the cache-hit output. However, especially for advanced workflows, this quickly becomes cumbersome to add every time. It's also an additional 3 line for each job and doesn't include a good error message.

  - name: Check cache hit
    if: steps.cache.outputs.cache-hit != 'true'
    run: exit 1

As mentioned in #955 (comment), the goal to simplify the configuration doesn't exclude the possibility to help with more advanced use cases. Including the new option will make it easier to use, help keep workflows more readable, and enable better error messages.

How Has This Been Tested?

Added test cases and run a test workflow using the feature branch.

Screenshots (if appropriate):

--

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation (add or update README or docs)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Sorry, something went wrong.

@cdce8p cdce8p requested a review from a team as a code owner December 22, 2022 16:09
@github-actions github-actions bot requested a review from lvpx December 22, 2022 16:10
@kotewar kotewar requested review from kotewar and removed request for lvpx December 26, 2022 04:51
@kotewar kotewar assigned kotewar and unassigned lvpx Dec 26, 2022
cdce8p added 3 commits January 8, 2023 10:38

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@cdce8p
Copy link
Contributor Author

cdce8p commented Jan 8, 2023

@kotewar I rebased all my PRs to resolve the merge conflicts and update the tests after the recent changes. Is there a timeline when this can move forward? It would be awesome if I wouldn't have to resolve merge conflicts every other week.

@cdce8p cdce8p requested review from bishal-pdMSFT and removed request for kotewar January 13, 2023 11:43
@kotewar kotewar mentioned this pull request Jan 30, 2023
10 tasks
@kotewar
Copy link
Contributor

kotewar commented Jan 30, 2023

Looks good to me, before we can release this out, we need a few additions

  1. Please update the patch version in package.json
  2. Please update RELEASES.md and the Whats new section of README.md with appropriate release info.

I'll approve, merge the PR and release the new version once the info is updated! Thanks :)

@kotewar
Copy link
Contributor

kotewar commented Jan 30, 2023

Whats new section of README.md with appropriate release info.

This also please. Just one liner is enough.

@cdce8p
Copy link
Contributor Author

cdce8p commented Jan 30, 2023

Whats new section of README.md with appropriate release info.

This also please. Just one liner is enough.

Sorry. Missed that the first time. Should be good now.

Copy link
Contributor

@kotewar kotewar left a comment

Choose a reason for hiding this comment

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

LGTM

@kotewar kotewar merged commit 627f0f4 into actions:main Jan 30, 2023
@kotewar
Copy link
Contributor

kotewar commented Jan 30, 2023

Thanks @cdce8p for your contribution, the changes are live 🚀

Please do test the positive and negative responses once with the latest tag actions/cache@v3 and actions/cache/restore@v3.

@cdce8p
Copy link
Contributor Author

cdce8p commented Jan 30, 2023

Thanks @cdce8p for your contribution, the changes are live 🚀

Awesome 🎉 Thanks for your help getting it ready.

Please do test the positive and negative responses once with the latest tag actions/cache@v3 and actions/cache/restore@v3.

actions/cache@v3.2.4 does seem to work as expected from my initial testing. However, the v3 tag doesn't seem to have been updated. It still points to 3.2.3 (58c146c) not 3.2.4 (58c146c).
Thus I just get Unexpected input(s) 'fail-on-cache-miss' for that.

@cdce8p cdce8p deleted the fail-on-cache-miss branch January 30, 2023 11:48
@kotewar
Copy link
Contributor

kotewar commented Jan 30, 2023

Let me check

@kotewar
Copy link
Contributor

kotewar commented Jan 30, 2023

My bad, I forgot to tag the version to v3. Please check now.

@cdce8p
Copy link
Contributor Author

cdce8p commented Jan 30, 2023

My bad, I forgot to tag the version to v3. Please check now.

It works now! Thanks

Dakshjain1 pushed a commit to SvavaCapital/cache that referenced this pull request Apr 19, 2024
* Add fail-on-cache-miss option

* Small improvements

* Changes after rebase

* Update description

* Only fail if no cache entry is found

* Code review

* Update readme

* Add additional test case

* Bump version + changelog

* Update package-lock.json

* Update Readme
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

optional action failure on cache miss
4 participants