Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gha: fix issue when caching virtual environments #62

Merged
merged 2 commits into from
Jun 18, 2022
Merged

Conversation

skshetry
Copy link
Member

@skshetry skshetry commented Jun 18, 2022

Whenever the setup-python action would provide a new patch version of the python (as it happened with 3.10.4 -> 3.10.5), it'd break the caching of the virtual environment in the nox and would throw following (cryptic) error:

Error: python is not installed into the virtualenv, it is located at /opt/hostedtoolcache/Python/3.10.5/x64/bin/python. Pass external=True into run() to explicitly allow this.

It seems it's because the virtualenv has a broken symlink to the old interpreter that does not exist after the update.

actions/setup-python#182 may be related.

With this change, the cache key now depends on the full python version instead of just major.minor. So this issue should hopefully not happen again, and automatically get invalidated.

Also see 855ea68, d4482b7 and d6606c9.

Whenever the `setup-python` action would provide a new patch version of the python (as it happened with 3.10.4 -> 3.10.5), it'd break the caching of the virtual environment in the nox and would throw following (cryptic) error:

```
Error: python is not installed into the virtualenv, it is located at /opt/hostedtoolcache/Python/3.10.5/x64/bin/python. Pass external=True into run() to explicitly allow this.
```

It seems it's because the virtualenv has a broken symlink to the old interpreter that does not exist after the update.

actions/setup-python#182 may be related.

With this change, the cache key now depends on the full python version instead of just major.minor. So this issue should hopefully not happen again, and automatically get invalidated.
@skshetry skshetry self-assigned this Jun 18, 2022
@codecov-commenter
Copy link

codecov-commenter commented Jun 18, 2022

Codecov Report

Merging #62 (5f8d881) into main (855ea68) will decrease coverage by 0.75%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main      #62      +/-   ##
==========================================
- Coverage   45.44%   44.68%   -0.76%     
==========================================
  Files          34       34              
  Lines        2007     2003       -4     
  Branches      238      213      -25     
==========================================
- Hits          912      895      -17     
- Misses       1077     1092      +15     
+ Partials       18       16       -2     
Impacted Files Coverage Δ
src/dvc_objects/fs/system.py 60.29% <0.00%> (-23.53%) ⬇️
src/dvc_objects/fs/utils.py 41.48% <0.00%> (-0.44%) ⬇️
src/dvc_objects/fs/path.py 0.00% <0.00%> (ø)
src/dvc_objects/executors.py 31.11% <0.00%> (+0.67%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 855ea68...5f8d881. Read the comment docs.

@skshetry skshetry merged commit cb3a5e5 into main Jun 18, 2022
@skshetry skshetry deleted the fix-cache branch June 18, 2022 11:29
@efiop
Copy link
Contributor

efiop commented Jun 18, 2022

Great research! Thank you, @skshetry !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants