From cf69ecf607667f66f0d92bdcd895f1f4840d8cd9 Mon Sep 17 00:00:00 2001 From: Tomas Fabrizio Orsi Date: Thu, 12 Dec 2024 17:16:38 -0300 Subject: [PATCH] fix(l1, l2, levm): move `loc_report.json` to `loc_report.json.old` even when there a `restore-keys` is used. (#1438) Even when there is no cache-hit, the file is fetched. If it came from the `restore-keys` directive, it doesn't count as a cache-hit even if the file is fetched. **Motivation** Show LOC diffs in the different slack channels. **Description** This description was taken from this comment: https://github.com/lambdaclass/ethrex/pull/1433#issuecomment-2524828313 We are saving the log_report.json file on a cache entry that has as key loc-report-${{ github.ref_name }}. github.ref_name is the name of a branch not a commit hash. That name is not "main" but rather the name of the PR it came from. In this case ci-diffs. What's the problem with that? If the branch names don't match EXACTLY, the cache fails. HOWEVER, we have a restore-keys as a backup (here: https://github.com/lambdaclass/ethrex/blob/main/.github/workflows/daily_reports.yaml#L124-L125). restor-keys is a backup that acts like a sort of regex. It tells github-cache "Hey, seen as you failed fetching that EXACT key, give me, as a backup, a value that matches the following prefix". That prefix should match everytime, since every key we generate uses that exact same prefix. So what's the problem? The problem is that we are only changing the file name IF there is a cache hit aka this if: https://github.com/lambdaclass/ethrex/blob/main/.github/workflows/daily_reports.yaml#L132. So, once again, what's the problem? We are getting a cache hit everytime, we either get it with the exact key or using the restore-keys as a fallback. And there is the problem, if the value is fetched using restore-keys, it doesn't count as a cache hit. This behavior is described here: https://github.com/actions/cache/blob/main/README.md?plain=1#L129. That would also explain why the script worked while I was working on this PR: I was always on the same branch, so it was always a cache hit, it was never using restore-keys. However, when the cron job is executed, it probably fails every time and ends up using the backup. That is also strange, one would think that it would end up using "main" branch as a key name. My hunch is that it uses commits coming from PR's, so it's maybe using those PR branch names for keys. I'm gonna create a new PR for it to be merged on Tuesday, so we can see the difference in output and check that this indeed is the issue. No associated issue number --- .github/workflows/daily_reports.yaml | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/.github/workflows/daily_reports.yaml b/.github/workflows/daily_reports.yaml index 94c885a21d..57d15e9c9f 100644 --- a/.github/workflows/daily_reports.yaml +++ b/.github/workflows/daily_reports.yaml @@ -124,12 +124,8 @@ jobs: restore-keys: | loc-report- - - name: Check previous loc report - run: | - cat loc_report.json - - name: Rename cached loc_report.json to loc_report.json.old - if: steps.cache-loc-report.outputs.cache-hit == 'true' + if: steps.cache-loc-report.outputs.cache-hit != '' run: mv loc_report.json loc_report.json.old - name: Generate the loc report