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

Clarify caching inputs #77

Merged
merged 2 commits into from
Jul 2, 2024
Merged

Clarify caching inputs #77

merged 2 commits into from
Jul 2, 2024

Conversation

memark
Copy link
Contributor

@memark memark commented Jun 30, 2024

The documentation mentions shared-key and cache-key as an input, but the latter doesn't seem to exist. I've changed the wording to refer to prefix-key and shared-key instead.

Copy link
Owner

@obi1kenobi obi1kenobi left a comment

Choose a reason for hiding this comment

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

Nice catch, thank you!

@obi1kenobi obi1kenobi enabled auto-merge (squash) July 1, 2024 13:51
@memark
Copy link
Contributor Author

memark commented Jul 2, 2024

Anything more I can do here? I'm assuming the failing CI tests are unrelated to my docs change.

obi1kenobi added a commit that referenced this pull request Jul 2, 2024
It's always caching. Or DNS. This time it was caching.

A while back, we changed the default cache key prefix from empty-string to `semver` so that our cache key names don't start with a dash, since users found that confusing.

We should have updated these tests accordingly at the time, but we did not. So how did the PR with the change pass?

It used the caches from older runs of the same job which used the old names. When those caches expired and were GC'd by GitHub, the jobs started failing.

This is what broke CI for #77. This PR will fix it.
@obi1kenobi
Copy link
Owner

Indeed. Thanks for the ping!

It's an interesting failure due to GitHub Actions caching that caused an older commit that should have failed CI to incorrectly pass CI by using a previous run's cache:
#79

After that PR merges, this CI should be green again. I'll keep an eye on it now and make sure it's all good.

obi1kenobi added a commit that referenced this pull request Jul 2, 2024
It's always caching. Or DNS. This time it was caching.

A while back, we changed the default cache key prefix from empty-string to `semver` so that our cache key names don't start with a dash, since users found that confusing.

We should have updated these tests accordingly at the time, but we did not. So how did the PR with the change pass?

It used the caches from older runs of the same job which used the old names. When those caches expired and were GC'd by GitHub, the jobs started failing.

This is what broke CI for #77. This PR will fix it.
@obi1kenobi obi1kenobi merged commit 3e1d1b2 into obi1kenobi:main Jul 2, 2024
29 checks passed
@memark memark deleted the patch-1 branch July 14, 2024 19:45
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.

2 participants