-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Revert cache to use github.ref
instead of github.sha
#5460
Revert cache to use github.ref
instead of github.sha
#5460
Conversation
Thanks for the quick fix and no bad feelings! |
@squidfunk This means that even for pull request merges the
After rereading that I figured that the pull request So I'm currently out of ideas, because I do think that - name: Set Date String
run: echo "date=$(date +'%Y-%m-%d')" >> $GITHUB_ENV
# ...
key: mkdocs-material-${{ env.date }}
# ... Sorry for the confusion once again 😞 |
Haha, no worries. Many thanks for taking the time and investigating, much appreciated. Should we revert the last commit? I'm not sure what to do here and would follow your recommendation. I'm not sure whether date is a good idea though. It might lead to flaky builds that resolve on the next day 😅 |
I'm not sure if I understand this sentence. The cache is being restored from the previous key with a matching prefix, therefore using
... therefore I dobut reverting to the previous commit is the way to go. Using the approach with a date would limit the amount of caches to 7 per week (assuming daily commits), and would allow the users to delete them, more easily, in case of any issues. The downside is of course the rather non-straightforward step inside of the workflow file. Another approach could be splitting the cache into restore and saving steps (which I ignored previously): uses: actions/cache/restore@v3
id: restore-cache # step id required to get the output, and use same hash
with:
restore-keys: mkdocs-material- Save approach uses: actions/cache/save@v3
with:
path: .cache
key: ${{ steps.restore-cache.outputs.cache-matched-key }} Save approach uses: actions/cache/save@v3
with:
path: .cache
key: mkdocs-material-${{ hashfiles('.cache/**') }} This would be the "cleanest" way to properly manage caches, but it once again adds more steps to copy / paste, and for the user to understand, in a way it's worse than the date. I'll see if I can manage to introduce it in a good way to the current documentation. |
Thanks again! I'm having a little hard time wrapping my head around this (currently fighting other battles), so I'll trust your recommendation on this. If you wish to follow up on this, feel free to take the necessary time to come up with a solution that is simple to follow for the user (= less maintenance for us maintainers because of fewer questions) and works reasonably well. Maybe someone else with more experience can share some ideas as well
As we both know, caching is hard. We had some problems a while ago with stale Python dependencies that were tricky to resolve. For this reason, I deactivated caching of pip-installed dependencies. Scoping caches to days introduces a new axis of fragility. When we run into a caching issue on day 1, and try to debug it on day 2, it might be hard to reproduce. When it's scoped to a ref, it's likely easier to debug, as we don't have time working against us. That's basically what I meant. I'm all for solutions that are simple, yet solve the widest range of problems while being easy to debug and extend. Maybe we can find such a solution here as well. |
Fixes my mistake from #5448.
I added a code annotation to the Publishing Guide, explaining when should the users expect a cache update.
I considered to add another sentence "If your project requires more frequent cache updates, then use
github.sha
", but I guess this is a too much of an edge-case, that people might just overuse "just in case". Also using a YYYY-MM-DD date instead would be better thangithub.sha
, because it would limit caches to 1 day.Sorry for the PR chaos 🙏😅