actions/cache key is not only about exact match

[Totktonada]

Code of Conduct

• [X] I have read and agree to the GitHub Docs project's Code of Conduct

What article on docs.github.com is affected?

https://docs.github.com/en/actions/advanced-guides/caching-dependencies-to-speed-up-workflows#matching-a-cache-key

What part(s) of the article would you like to see updated?

The documentation provides description of key and restore-keys fields. The actual behaviour is the following:

1. If key match existing cache entry exactly, the cache is restored and cache-hit output value is set to 'true'.
2. If key match a prefix of an existing cache entry, the cache is restored and cache-hit output value is set to ''.

The latter is not obvious from the documentation. To be honest, I would even say that the article reads like it works in the opposite way.

The reason why I misunderstood the documentation is the following. The description of the key does not say anything about exact/prefix match. It also say that match is called 'cache hit', so, I guess, it is when cache-hit is set to 'true': exact match.

Next, restore-keys description differentiates 'exact match' and 'partial match' (prefix match in fact). So my guess was that if key would behave this way, it would be described as the common behaviour of both fields. Now it looks like a highlight that restore-keys works differenctly from key, but that's not so.

I would also highlight that cache-hit value is not described. It is quite non-obvious that a cache may be restored, but the output value will give '', because it is not exact match.

Additional information

One can verify the behaviour youself. Let's create a workflow file with the following content and push it to a branch:

name: Test cache

on: [push]

jobs:
  test_cache:
    env:
      CACHE_KEY: my-cache-key-v1-draft
    runs-on: ubuntu-latest
    steps:
      - name: Cache file
        uses: actions/cache@v2
        id: cache
        with:
          path: cached_file
          key: ${{ env.CACHE_KEY }}

      - name: Generate file
        run: |
          [ ! -f cached_file ]  # should never give an error
          printf 'CACHE_KEY: %s\n' "${{ env.CACHE_KEY }}" > cached_file
        if: steps.cache.outputs.cache-hit != 'true'

      - name: Show file
        run: |
          cat cached_file

Next, change the cache key:

diff --git a/.github/workflows/test_cache.yml b/.github/workflows/test_cache.yml
index 5a0f6d4..a3d448e 100644
--- a/.github/workflows/test_cache.yml
+++ b/.github/workflows/test_cache.yml
@@ -5,7 +5,7 @@ on: [push]
 jobs:
   test_cache:
     env:
-      CACHE_KEY: my-cache-key-v1-draft
+      CACHE_KEY: my-cache-key-v1
     runs-on: ubuntu-latest
     steps:
       - name: Cache file

And push it to a branch. What will occur? The cache will be restored from my-cache-key-v1-draft, steps.cache.outputs.cache-hit will be '', so we'll run the 'Generate file' step and get a failure.

In fact, we made two mistakes here:

1. Assume that key is about exact match.
2. Assume that if cache restore occurs it means that cache-hit will be 'true'.

Those pitfalls should at least be highlighted in the documentation. However, to be honest, those are good candidates to revisit in a next major version of the action.

---

Content Plan

Guidance here

[welcome[bot]]

Thanks for opening this issue. A GitHub docs team member should be by to give feedback soon. In the meantime, please check out the contributing guidelines.

[ethomson]

Hi @Totktonada ! Thanks for opening this issue. At the moment, the documentation states:

cache-hit: A boolean value to indicate an exact match was found for the key.

Can you suggest how we could clarify this? Feel free to open a PR and we can iterate on the language there, if you'd like!

[hosein1arti]

14145

[Totktonada]

The cache-hit output parameter description is technically correct, however I would specifically highlight the 'exact' word or explicity mention that a partial match sets the parameter to ''.

The main problem is not here. Let's look at the following paragraph:

You can provide a list of restore keys to use when there is a cache miss on key. You can create multiple restore keys ordered from the most specific to least specific. The cache action searches for restore-keys in sequential order. When a key doesn't match directly, the action searches for keys prefixed with the restore key. If there are multiple partial matches for a restore key, the action returns the most recently created cache.

The whole paragraph is about restore-keys. It highlights that the cache action searches for the keys prefixed with given ones. Is it about restore-keys or both key and restore-keys? I read it as the former, but actual behaviour is the latter.

Next, the list below:

1. If you provide restore-keys, the cache action sequentially searches for any caches that match the list of restore-keys.
   • When there is an exact match, the action restores the files in the cache to the path directory.
   • If there are no exact matches, the action searches for partial matches of the restore keys. When the action finds a partial match, the most recent cache is restored to the path directory.
2. <...>
3. <...>

The same problem. The logic about prefixing is described for restore-keys, so I read it as if key would have different logic.

---

I would prefer to leave a feedback and go ahead, but if the problem still not clear enough, I'll open a pull request.

[github-actions[bot]]

This is a gentle bump for the docs team that this issue is waiting for technical review.

[Kieudung717]

PowerGửi từ Galaxy của tôi -------- Tin nhắn gốc --------Từ: "github-actions[bot]" @.> Ngày: 13/04/2022 03:48 (GMT+07:00) Đến: github/docs @.> Cc: Subscribed @.***> Chủ đề: Re: [github/docs] actions/cache key is not only about exact match (Issue #14145) This is a gentle bump for the docs team that this issue is waiting for technical review.

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you are subscribed to this thread.Message ID: @.***>

[Kieudung717]

??????

Vào 03:48, T.4, 13 Th4, 2022 github-actions[bot] @.***> đã viết:

This is a gentle bump for the docs team that this issue is waiting for technical review.

— Reply to this email directly, view it on GitHub https://github.com/github/docs/issues/14145#issuecomment-1097199359, or unsubscribe https://github.com/notifications/unsubscribe-auth/AXTWXQIWWZE3346SI3QT63DVEXOP3ANCNFSM5MBNN5FA . You are receiving this because you are subscribed to this thread.Message ID: @.***>

[ramyaparimi]

@Totktonada Thanks so much for your patience. I spoke with the team and we are not going to accept this contribution :yellow_heart:. We really appreciate your time and interest in improving GitHub docs 💖 .

[Totktonada]

@ramyaparimi Can you reveal a reason, please?

[ramyaparimi]

@Totktonada I am extremely sorry for closing this issue. I didn't notice the comment with an SME review earlier and closed it assuming it was not reviewed by an SME. I apologize for this and I am going to reopen this issue and get this triaged for a writer's review 👀.

Thanks so much for your patience and understanding 💖.

[github-actions[bot]]

Thanks for opening an issue! We've triaged this issue for technical review by a subject matter expert :eyes:

[ramyaparimi]

@Totktonada I have triaged this for a subject matter expert to review the approach in this comment. Thanks so much for your patience 💖 .

[github-actions[bot]]

This is a gentle bump for the docs team that this issue is waiting for technical review.

[janiceilene]

👋 @Totktonada Thanks so much for your patience as the team reviewed your suggestions!

You (or anyone else!) are welcome to open a PR making the changes described in https://github.com/github/docs/issues/14145#issuecomment-1028540700 💖

[felicitymay]

Hi @Totktonada 👋🏻

We're reviewing all open help wanted issues as part of our preparation for Hacktoberfest, which is coming up shortly.

It looks as if this article and the information about cache-hit was updated in November 2022 to improve the clarity.

We'll plan to close this issue at the end of the week unless you have further feedback on this article.

[Totktonada]

@felicitymay, hi!

I guess you're about 956918468feeca5198323fd8274e71993f8e8a53. As I see, the information about key, restore-keys and cache-hit is still incomplete.

In my understanding (if nothing was changed in the cache action implementation), the documentation should make clear the following points.

• key allows a partial match.
• cache-hit is set to '' (not 'true') at the partial match.

[felicitymay]

Thank you for getting back to us so quickly with clear feedback. I'm not an actions expert, so this is really helpful. 💖

I'll update the issue summary soon and this will be ready for someone to work on. We usually get a lot of contributions during Hacktoberfest.

[felicitymay]

Before updating the content plan for fixing the documentation, I spent some time looking at the documentation for the cache action. I'm now wondering if the behavior you observed is the intended behavior.

I'm going to check with the action maintainers because the partial matching behavior on key you see doesn't seem to be covered in the docs or tests for the cache action.

[Totktonada]

Thank you!

I would highlight one related point.

I don't remember precisely now, but even if the key behavior will be changed and/or documented, restore-keys likely also set cache-hit to '' on the partial match and it may be not obvious behavior.

We can't lean on ${{ steps.mycachestep.outputs.cache-hit != 'true' }} as a criteria for a cache miss. I think that the documentation should provide three criterias: cache hit with exact match (== 'true'), cache hit with exact or partial match, cache miss.

[felicitymay]

Hi @Totktonada. Thank you for your patience with this discussion and for all your thoughtful feedback. ✨

I've just had a discussion with a member of the team that is taking over support of this action. They agree that the behavior you've uncovered might well be unintended. We're going to look into the behavior internally and update this article once we've corrected any problems with the action itself. I'm going to close this issue as we will track both the action and documentation updates internally.

Thank you for all your help identifying this inconsistency 🙇🏻‍♀️