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 cache-prefixing #1169

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Add cache-prefixing #1169

wants to merge 4 commits into from

Conversation

echang49
Copy link

@echang49 echang49 commented Aug 2, 2024

Description:
This changes allow the state saved to the global action cache to be stored with a prefix. Useful as _state is not a unique name and multiple workflows could save the cache as _state. Also useful as you currently cannot use this action twice for independent runs as the runs would pick up the same state. With this change, it is possible to have one run close all PRs with label x after 7 days and another run close all PRs except those with label x after 10 days.

Also updated the README.md to show proper permissions needed.

Related issue:
#1137

Please also look into merging PR #1152 as this isn't necessarily useful if caching isn't 100% fixed.

Check list:

  • Mark if documentation changes are required.
  • Mark if tests were added or updated to cover the changes.

@echang49 echang49 requested a review from a team as a code owner August 2, 2024 02:09
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
echang49 and others added 2 commits August 3, 2024 22:47
Co-authored-by: ccoVeille <[email protected]>
Co-authored-by: ccoVeille <[email protected]>
@VOVELEE
Copy link

VOVELEE commented Aug 28, 2024

I am hitting similar issue but this time it is because I have couple of runners working on the same VM. In this situation the user (under which the first runner operates) creates the folder and file and remains owner preventing other runners (running under different context) to have access to the tmp folder.

@autoantwort
Copy link

This solves the same problem as #1134. Here the user has to do manually work so solve this problem, with #1134 the user don't have to do anything. With this PR changes to the action configuration don't restart the process, with #1134 this will reset the progress.

@echang49
Copy link
Author

echang49 commented Nov 27, 2024

This solves the same problem as #1134. Here the user has to do manually work so solve this problem, with #1134 the user don't have to do anything. With this PR changes to the action configuration don't restart the process, with #1134 this will reset the progress.

It's been a while so the full reasoning escapes me. I remember seeing your PR and the reasoning why I created a separate one was because:

  • Not a fan of having hashed cache keys. Especially in the presence of other CI utilizing caches. One reason is that it becomes harder to determine where a cache is coming from. The use case that I had, a repository regularly exceeded the allowed cache limit. While some caches were ok to evict, others (like this one), should have been kept. With hashed cache keys, it becomes difficult to know which caches should be evicted or not.
  • Seems like the possibility of having collisions with caching is still present if parameters are still present (if the params are all the same). I wanted the user to decide if they wanted to use the same cache or a different one. Didn't want to remove the choice from them. NOTE: I'm not 100% on this, that's just what it seems like from the updated getStateInstance function in github actions cache: Use more unique key #1134

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.

4 participants