setup-python icon indicating copy to clipboard operation
setup-python copied to clipboard
verified publisher icon actions

fix: add arch to cache key

Open Zxilly opened this issue 1 year ago • 10 comments

Description:

GitHub has added the arm64 runner, so arch should be used as part of the cache key. Otherwise, if there are macos-13 macos-14 running at the same time, the late executor will get the wrong cache.

This problem is particularly acute for pipenv and poetry, for which action also caches the contents of venv. This prevents any libraries containing native binary dependencies from working properly.

For real-world error, see https://github.com/Zxilly/go-size-analyzer/actions/runs/9564444574/job/26365211564

Check list:

  • [ ] Mark if documentation changes are required.
  • [x] Mark if tests were added or updated to cover the changes.

Zxilly avatar Jun 18 '24 12:06 Zxilly

@actions/setup-actions-team Can anyone review this?

Zxilly avatar Jun 19 '24 12:06 Zxilly

@actions/setup-actions-team I'm wondering if anyone is still maintaining this project, between the fact that GitHub has started to offer the Ubuntu arm64 runner, this problem is getting worse.

Zxilly avatar Jul 13 '24 17:07 Zxilly

@HarithaVattikuti @priya-kinthali Can you take a look on this?

Zxilly avatar Jul 13 '24 17:07 Zxilly

Hello @Zxilly, Thank you for this PR, we are looking into it and we will get back to you once we have some feedback on this :)

aparnajyothi-y avatar Jul 19 '24 05:07 aparnajyothi-y

Hello @Zxilly, We have reviewed and tested the changes from our end for Including arch in the cache key has made the run successful. However, we are unable to see arch in the cache key with all the changes of this PR. Please find the screenshots for reference. Please have a look at this PR and update to have this to be approved and merged. Screenshot 2024-07-24 at 5 38 53 PM Screenshot 2024-07-24 at 5 42 15 PM

aparnajyothi-y avatar Jul 25 '24 06:07 aparnajyothi-y

@aparnajyothi-y Apparently the cache in the image is an npm cache created by setup-node, does it have anything to do with this PR? This repo is setup-python

Zxilly avatar Jul 25 '24 06:07 Zxilly

I think setup-node has a similar PR https://github.com/actions/setup-node/pull/843 and got no review for a long time.

Zxilly avatar Jul 25 '24 06:07 Zxilly

Hello @Zxilly, Please find attached the setup-python cache key screenshots for reference where the changes of Including arch in the cache key has made the run successful. However, we are unable to see arch in the cache key with all the changes of this PR. Please have a look at this PR and update to have this to be approved and merged.

image image

aparnajyothi-y avatar Jul 25 '24 09:07 aparnajyothi-y

You use the https://github.com/aparnajyothi-y/setup-python/commits/PR%23896-test/ workflow.

The branch you created only contains changes to the ts file, but GitHub Actions executes the js file, and you should compile the ts file to js to allow it to work properly.

Zxilly avatar Jul 25 '24 09:07 Zxilly

If you want to test this PR, you should directly use Zxilly/setup-python@main

Zxilly avatar Jul 25 '24 09:07 Zxilly

Hello @Zxilly, Could you please confirm that the code has passed the checks executed by npm run format-check and npm test? We are currently unable to run / view the checks on this PR and need confirmation to proceed with merging this PR. Thank you.

aparnajyothi-y avatar Aug 06 '24 13:08 aparnajyothi-y

image image

All passed.

Zxilly avatar Aug 06 '24 13:08 Zxilly

I'm curious, why don't you execute the tests yourself? It's unimaginable that such a simple PR took almost two months.

Zxilly avatar Aug 06 '24 14:08 Zxilly