-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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 caching to lychee links checker #5160
Conversation
I'm currently basing the cache on the GIT commit, but we can make it global if needed. |
Won't caching on the git sha mean a new cache is used for each commit? That will make the cache ineffective as it will be new for each commit, right? |
Based on the lychee and actions/cache documentations, I don't think that's accurate. We do store the sha in the cache name, but if there is no cache hit, the |
Discussing with @pellared, we have decided to only cache if the run is fully successful, because lychee caches everything, including failures. |
Co-authored-by: Robert Pająk <pellared@hotmail.com>
Doesn't this mean that we will always use the last cache from "restore-keys" in the normal operation? We almost never re-run the link checker on the same commit. If this is the case using a global cache would be equivalent and reduce the number of caches stored in actions. |
Yes
Can you please describe what do you mean by "a global cache"? Do you mean using a single cache key value? The problem with it is that the action does not save an updates the cache when the cache was restored using the key. See #5160 (comment) I think this is the main reason why lychee recommends such way of caching. |
👍 |
Waiting for an non-Splunker approval. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't looked too deeply into lychee, but approving since this follows https://github.com/lycheeverse/lychee-action?tab=readme-ov-file#utilising-the-cache-feature, and based on other approvals.
A few months ago, we enabled lychee cache in the SDK repo, to prevent rate limit issues, but forgot to enable it here. open-telemetry/opentelemetry-go#5160 --------- Co-authored-by: Robert Pająk <pellared@hotmail.com>
This adds caching to the links checker, to hopefully reduce the false positive failures due to 429s.