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

adjust key computation to avoid trailing - #186

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

compnerd
Copy link
Contributor

The - suffix was being applied unconditionally resulting in a hanging - suffix if no timestamp was used. Adjust the key computation to remove the trailing - if no timestamp is used.

The `-` suffix was being applied unconditionally resulting in a hanging
`-` suffix if no timestamp was used. Adjust the key computation to
remove the trailing `-` if no timestamp is used.
@hendrikmuhs
Copy link
Owner

The change also affects restore keys. I am a bit afraid of unwanted side effects. It looks like a breaking change at the moment.

What about only removing the - or appending no_timestamp or something similar?

@compnerd
Copy link
Contributor Author

What about only removing the -

Hmm, this was the effect that I was going for. Perhaps I misunderstood the code and did something incorrectly for accomplishing the desired effect?

appending no_timestamp or something similar?

That does still add a suffix which does make it harder to find the cache entry when you have a number of them unfortunately :(.

const primaryKey = inputs.primaryKey ? keyPrefix + inputs.primaryKey + "-" : keyPrefix;
const restoreKeys = inputs.restoreKeys.map(k => keyPrefix + k + "-")
const primaryKey = inputs.primaryKey ? ccacheVariant + "-" + inputs.primaryKey : ccacheVariant;
const restoreKeys = inputs.restoreKeys.map(k => ccacheVariant + "-" + k)
Copy link
Owner

Choose a reason for hiding this comment

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

@compnerd

the concrete line I talk about. I understand that before the change the restore key is:

"ccache-{k}-"

after

"ccache-{k}"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, so, I made that change because I changed the save key. If that change is not needed, I'm totally happy to undo it. Maybe I didn't fully grok how the save/restore keys work.

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