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

add pipenv to image #39

Closed
wyardley opened this issue Jun 18, 2020 · 7 comments · Fixed by #40
Closed

add pipenv to image #39

wyardley opened this issue Jun 18, 2020 · 7 comments · Fixed by #40

Comments

@wyardley
Copy link

wyardley commented Jun 18, 2020

Since pipenv is pre-installed on the old Python images, and (I believe) is still the documented way to handle image caching (https://circleci.com/docs/2.0/language-python/#cache-dependencies), would it make sense to bake it into this image as well? The example documented here also won't work (sudo pip install pipenv), because pip isn't in root's path (#19 (comment))

I tried switching from circleci/python to cimg/python and noticed that it failed because of it not being installed. While it's a relatively simple step to install it, it would be convenient to have it there.

@wyardley wyardley changed the title pipenv add pipenv to image Jun 18, 2020
@FelicianoTech
Copy link
Contributor

Great question. This image uses pyenv to obtain the Python version. In theory, this shouldn't interfere with Pipenv right?

I'll do some testing and research and see if we can add this in before this image goes stable/GA, which is soon.

@wyardley
Copy link
Author

Right - they’re supposed to work ok together, and I think newer pipenv may even work with pyenv.

FWIW, just doing pip install pipenv (without sudo) seems to work fine, so it may be as simple as adding it to the pip packages that get installed in the base image.

Happy to PR it but wasn’t sure exactly where that goes in the stuff that generates the Dockerfiles and whether to commit the results as well

@dsayling
Copy link
Contributor

dsayling commented Jun 18, 2020

Hey there! I've been thinking about this problem myself. There are a few things we just need to make sure are clear and we discuss before making the change. I do believe its fair to assume that pipenv should be the default packaging tool available in the base image since pypa officially recommends pipenv.

Should we hardcode a version of pipenv to install?
Pipenv has had issues with releases breaking installs. The last version released before 2020* was 2018*. Two years was quite a while and who knows what an unexpected update will cause

Along with pipenv will come a few other packages - do their versions matter / will they update ok without issue?
Currently there are no python packages in pyenv site-packages in the images (except for pip). In the 3.7 image today if I install pipenv (at version 2020.6.2), I also install these packages. Should we set their versions as well?

  • appdirs-1.4.4
  • certifi-2020.4.5.2
  • distlib-0.3.0
  • filelock-3.0.12
  • importlib-metadata-1.6.1
  • six-1.15.0
  • virtualenv-20.0.23
  • virtualenv-clone-0.5.4
  • zipp-3.1.0

@wyardley
Copy link
Author

FWIW, I think the regular Circle images are already getting the 2020 version of pipenv, for better or for worse:

LM-LTH-40012925~% docker run circleci/python:3.7 pipenv --version    
pipenv, version 2020.6.2

so, to me, jumping backwards on the cimg ones wouldn't make a ton of sense?

For our workflows, we've seen one or two issues, but the new version also fixes a bunch of stuff

@dsayling
Copy link
Contributor

Definitely not proposing we go backward! Sorry for not being clear. I'm basically wondering if the change should be
pip install pipenv or pip install pipenv==2020.6.2 or lock all the dependent pip pacakges for pipenv into a requirements file with every version hardcoded. I'm not quite sure what the planned release cadence for https://github.com/pypa/pipenv is going to be. Without hardcoding the version - unexpected new versions of pipenv or the dependent packages may be installed when the image is rebuilt. Just trying to discuss the risk to the change long term.

@dsayling
Copy link
Contributor

FWIW - if you want to make the change and feel comfortable making the change to the https://github.com/CircleCI-Public/cimg-python/blob/master/Dockerfile.template. The README should provide guidance on testing the change and submitting the PR.

@wyardley
Copy link
Author

I do kind of wonder what the benefit of using pyenv in a container that's already using a single Python version is... unless it makes using the same Dockerfile for all the versions that much easier -- from a simplicity standpoint, might be easier / better to install at system level, and then there's less likely to be weird path issues.

I am seeing binaries installed from pipenv missing from the pyenv path when I tried switching to this, but maybe I need to do some additional pyenv configuration in the pipenv config...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants