fix(l1, l2, levm) #1433
fix(l1, l2, levm) #1433
Conversation
lima-limon-inc
force-pushed
the
ci-diffs
branch
from
2cb66c2 to
53f1562
Compare
Bash tries to execute the content in between them; and we are simply trying to print
lima-limon-inc
force-pushed
the
ci-diffs
branch
from
e18d146 to
27be3ff
Compare
lima-limon-inc
force-pushed
the
ci-diffs
branch
from
27be3ff to
2cb3169
Compare
This reverts commit 53f1562.
lima-limon-inc
force-pushed
the
ci-diffs
branch
from
349bdc2 to
8d6abde
Compare
lima-limon-inc
force-pushed
the
ci-diffs
branch
from
5baaedb to
c79613d
Compare
| @@ -157,7 +158,6 @@ jobs: | |||
| - name: Post results to levm slack channel | |||
| env: | |||
| url: ${{ secrets.LEVM_SLACK_WEBHOOK }} | |||
| run: sh .github/scripts/publish_loc.sh | |||
There was a problem hiding this comment.
why are we removing this line? isn't it needed?
There was a problem hiding this comment.
Yes! That's why I was getting an error. I deleted the line by mistake.
There was a problem hiding this comment.
Fixed here: ef7bd71
Reported-by: Federico Borello <[email protected]>
This reverts commit 1e488ef.
This reverts commit 013963c.
lima-limon-inc
force-pushed
the
ci-diffs
branch
from
f0f5d2e to
3582e5c
Compare
lima-limon-inc
force-pushed
the
ci-diffs
branch
from
3582e5c to
7e10f0a
Compare
| - name: Check previous loc report | ||
| run: | | ||
| cat loc_report.json | ||
|
|
There was a problem hiding this comment.
I added this directive here to read the loc_report.json that was saved in the cache.
|
@ilitteri I think I found the problem. I think the problem is in this line: https://github.com/lambdaclass/ethrex/blob/main/.github/workflows/daily_reports.yaml#L132 We are saving the What's the problem with that?
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 And there is the problem, if the value is fetched using 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 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. |
|
@lima-limon-inc please in the future, add a more descriptive title. |
Noted. I apologize @mpaulucci for the inconvenience. |
…en 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. <!-- Why does this pull request exist? What are its goals? --> **Description** This description was taken from this comment: #1433 (comment) <!-- A clear and concise general description of the changes this PR introduces --> 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. <!-- Link to issues: Resolves #111, Resolves #222 --> No associated issue number
Motivation
Fix the loc test done in
daily_reports.yaml. ATTOW it does not include line changes.Description
The loc directive should send a message to the slack channel showing lines added and removed. This implied changes to the CI.
No issue for this PR.