Add boolean input variables to reevaluate primary cache key & to only restore (skip save post run) #826
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* implement only-restore within save
* add regular & proxy server jobs * run job between save & restore jobs to test that only-restore option works with consecutive restores
* also format code with prettier
Add boolean inputs to: * reevaluate cache's primary key * only restore the cache (not save it)
* could be eliminated if primary key is generated differently, e.g. hashing of file paths on reevaluation
* add logic to not save state in restore step when setting `reeval` to true * create infrastructure to set `Inputs.Reeval` input variable within tests * `Inputs.Reeval` set to false for all save & restore tests
|
Hi @prncevince 👋🏽 We have had a few similar feature requests/ PRs in the past. We are trying to consolidate all such requests and come up with a common solution. Your contribution will definitely help move this request forward. Please allow us some time to circle back on this PR while we evaluate the possible solutions. |
|
Thanks & Good luck! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I got a little ambitious with this one. Understand it may never get merged. This tackles a few PRs / issues & topics of conversation all in one:
On the topic of skipping the post run save script, aka only restoring:
only-restore. Default is set tofalse.On the topic of reevaluating the primary cache key:
reeval. Default is set tofalse.Being that I added new input variables, I had to do quite a bit to get all checks to pass. The goal was adding reevaluation of the primary key during the post run save script as a capability, which was accomplished. In a nut shell, for reevaluation, you need logic to handle when the primary key gets saved to the cache's state. This PR handles doing that.
The jest tests were edited bare minimally to pass. New jest tests were not added for say, testing setting boolean inputs of
only-restoreandreevalto eithertrueorfalse. However, test utilities were added to be able to read these input boolean variables. Also,Inputs.Reevalwas set tofalsethroughout both save and restore jest tests to evaluate save & restore scripts appropriately with the defaultreevalaction input value.The 'Tests' workflow was however changed to begin to handle setting the boolean variables to either.
The new field of
reevalwas added to the job matrix of thetest-save&test-restorejobs. In addition, matrices were added to thetest-proxy-save&test-proxy-restorejobs to handle this input as well. To differentiate primary keys between matrix runs, a-${{ matrix.reeval }}partial was appended to these keys.In addition, to handle the
only-restorecase, new jobs oftest-only-restore&test-proxy-only-restorewere created, each settingreevaltotrue, and their setting of the primary key follows the convention described above.I created a tag on the main branch of my own repo with these features as well. I've also tested it in quite a few other repos where it's used within composite actions.
Examples using the prncevince/r-actions/setup-knitr-cache composite action:
reevalset totrue:only-restoreset totrue:I also added to the README.md & to examples.md. To the readme I added descriptions of the input variables with their defaults. To the examples I added an example for using knitr caching when rendering R Markdown, as well as detailed explanation of why you would want to use it, with links to examples mentioned above.
A lot of this implementation could be better. I think that the creation of the primary keys during the Tests workflow could be based on an actual file hashing. This could be checked into the repository - say the __test__/__fixtures__/helloWorld.txt). Then, edits to the file could be reflected in the primary key and indeed be tested upon reevaluation.
I'm sure better messaging could be implemented with only restoring (forgoing the post run save script). Not sure that I'm going to pursue this much. Full implementation works right now on my own tag. But if what's requested by reviewers is simple enough for me to have a go to get it merged, I may be interested.
Cheers