Skip to content
This repository has been archived by the owner on Feb 23, 2022. It is now read-only.

Add pipenv and yarn caching #327

Merged
merged 28 commits into from
Mar 3, 2020
Merged

Add pipenv and yarn caching #327

merged 28 commits into from
Mar 3, 2020

Conversation

JackWilb
Copy link
Member

@JackWilb JackWilb commented Feb 21, 2020

Closes #323

@JackWilb JackWilb marked this pull request as ready for review February 21, 2020 22:44
@JackWilb JackWilb requested review from waxlamp and jjnesbitt and removed request for waxlamp February 21, 2020 22:44
@JackWilb
Copy link
Member Author

JackWilb commented Feb 21, 2020

To get this working, I've set the environment variable PIPENV_VENV_IN_PROJECT=1 and added an exclusion for the folder it makes, .venv in the flake8 config. Let me know if that seems reasonable or if you want me to try figure out the default install directory to cache.

Also, the pipenv cache caches the whole install python 3 install, which might not be ideal. Additionally, it only saves about 20%-40% of the install time so it might be simpler to let it install fresh each time.

@waxlamp
Copy link
Collaborator

waxlamp commented Feb 21, 2020

To get this working, I've set the environment variable PIPENV_VENV_IN_PROJECT=1 and added an exclusion for the folder it makes, .venv in the flake8 config. Let me know if that seems reasonable or if you want me to try figure out the default install directory to cache.

I'd prefer not to do this. Instead you can just run pipenv --venv in the project's top-level and it will tell you the path to the virtual environment.

Also, the pipenv cache caches the whole install python 3 install, which might not be ideal. Additionally, it only saves about 20%-40% of the install time so it might be simpler to let it install fresh each time.

What's the problem with caching the Python 3 install? Disk space is not at a premium is it?

@JackWilb
Copy link
Member Author

To get this working, I've set the environment variable PIPENV_VENV_IN_PROJECT=1 and added an exclusion for the folder it makes, .venv in the flake8 config. Let me know if that seems reasonable or if you want me to try figure out the default install directory to cache.

I'd prefer not to do this. Instead you can just run pipenv --venv in the project's top-level and it will tell you the path to the virtual environment.

Sure, I'll give it a go.

Also, the pipenv cache caches the whole install python 3 install, which might not be ideal. Additionally, it only saves about 20%-40% of the install time so it might be simpler to let it install fresh each time.

What's the problem with caching the Python 3 install? Disk space is not at a premium is it?

It was mentioned in the pipenv issue on the caching action that it might cause unknown errors since it's not just the packages that get cached, it's the whole python env.

@waxlamp
Copy link
Collaborator

waxlamp commented Feb 25, 2020

Also, the pipenv cache caches the whole install python 3 install, which might not be ideal. Additionally, it only saves about 20%-40% of the install time so it might be simpler to let it install fresh each time.

What's the problem with caching the Python 3 install? Disk space is not at a premium is it?

It was mentioned in the pipenv issue on the caching action that it might cause unknown errors since it's not just the packages that get cached, it's the whole python env.

Need some more detail on this. Can you quote the appropriate discussion or put a link here? "Unknown errors" sounds ominous, and not a place I want to go with out software.

@JackWilb
Copy link
Member Author

JackWilb commented Feb 25, 2020

This is from the issue I linked to in #323 :

I really dislike the virtualenv cache, since it's caching a lots of extra stuff, not only the dependencies. I am not sure what (terrible?) consequences this may cause, but I haven't had any issues with this settings in the past month

Let me know what you think

@jjnesbitt
Copy link
Member

I'd say it mostly looks good. For the python caching, I think it'd be good to follow the pattern used the issue you linked:

key: ${{ runner.os }}-${{ env.PYTHON_VERSION }}-virtualenvs-${{ hashFiles('Pipfile.lock') }} 

Which protects the CI from updates to the minor version of python.

@JackWilb
Copy link
Member Author

JackWilb commented Mar 2, 2020

I'd say it mostly looks good. For the python caching, I think it'd be good to follow the pattern used the issue you linked:

key: ${{ runner.os }}-${{ env.PYTHON_VERSION }}-virtualenvs-${{ hashFiles('Pipfile.lock') }} 

Which protects the CI from updates to the minor version of python.

I had added this before, but using the ${{ env.PYTHON_VERSION }} variable in the code wasn't producing any output. The cache was just linux--virtualenvs, no python version

@jjnesbitt
Copy link
Member

I had added this before, but using the ${{ env.PYTHON_VERSION }} variable in the code wasn't producing any output. The cache was just linux--virtualenvs, no python version

Gotcha. In that case I think it looks good.

@JackWilb JackWilb merged commit b1a3f84 into master Mar 3, 2020
@JackWilb JackWilb deleted the ci-caching branch March 3, 2020 17:46
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement yarn and pipenv caching for the CI
3 participants