cache icon indicating copy to clipboard operation
cache copied to clipboard

`cache-hit` needs to be replaced

Open jshier opened this issue 9 months ago • 4 comments

According to #1514, cache-hit is true when the cache is restored from the full key, false when the cache is restored from a fallback key, and empty when the cache wasn't hit at all. First, please merge this documentation update, as it means most usage of this value is wrong (few people care if the fallback key was used, quite a lot of people care when the cache wasn't hit). Second, this needs to be a different API, as a Bool is inappropriate for a three part value. Instead it should be replaced with something like cache-status: found | fallbackUsed | notFound, so users can be completely clear about what's happening.

jshier avatar Mar 06 '25 17:03 jshier

I just spent an hour or so debugging an issue I thought was due to my YML code being incorrect, but in fact it was correct and I was getting a cache miss (and cache-hit was the empty string).

It is really, really confusing UX to:

  1. Use a boolean for a three-part value.
  2. Return cache-hit as false when a restore-keys is matched .. because a cache was actually found.

Can one of the contributors please merge PR #1514?

@joshmgross @kotewar @dhadka @lvpx @takost

PaulRBerg avatar Apr 16 '25 12:04 PaulRBerg

And while we are at it, restore-keys should be renamed to something else.

PaulRBerg avatar Apr 16 '25 12:04 PaulRBerg

And while we are at it, restore-keys should be renamed to something else.

Dream on.

This would break a gazillion of workflows without good reason.

andreasabel avatar May 31 '25 06:05 andreasabel

Luckily they can version releases so that's not a problem. Or just deprecate the key while adding a more accurate one.

jshier avatar May 31 '25 06:05 jshier