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 support for pipenv #214

Closed
goern opened this issue Sep 26, 2017 · 17 comments
Closed

add support for pipenv #214

goern opened this issue Sep 26, 2017 · 17 comments
Assignees

Comments

@goern
Copy link
Contributor

goern commented Sep 26, 2017

Could supporting pipenv be added? :)

@mcyprian
Copy link
Collaborator

mcyprian commented Oct 2, 2017

Pipenv is higher-level Python packaging tool officially recommended from Python.org. Pip and virtualenv are integrated into this tools and it supports Pipfile which intends to replace requirements.txt in the future.
Support of pipenv is in my opinion a valid request. If you agree @torsava @hhorak @GrahamDumpleton, I will prepare a PR.

@hhorak
Copy link
Member

hhorak commented Oct 2, 2017

It's still not clear to me what changes are required in order to address this request. We don't have it packaged in RPMs, right? Would it be installed using pip and then used instead of pip?

@torsava
Copy link
Member

torsava commented Oct 2, 2017

@hhorak We don't have it packaged in RPMs, right? Would it be installed using pip and then used instead of pip?

Yes, that's the way we would have to go.


I'm not opposed to the idea, pipenv does bring some ease-of-use for developers. However, it might take some finesse to make it work here, since we're explicitly using virtualenv and pipenv wants to manage that on its own.

@mcyprian Maybe you can look into it and figure out how troublesome it would be to incorporate here. If there's a clean and easy solution without bad hacks, I'm all for it.

@hhorak
Copy link
Member

hhorak commented Oct 2, 2017

I agree, let's see how it could be implemented and then we can decide..

As a side-note, there can be more requirements like this that might be not easily solved in the generally available assemble script, and we'll simply need to offer users to extend functionality of the assemble scripts on their own -- one example of such functionality is #216. So, IOW, all the s2i images should have assemble script that allows extending by users. Once we have it and the pipenv support would be too messy, we can just document how to do it by extending the assemble script.

@mcyprian
Copy link
Collaborator

mcyprian commented Oct 2, 2017

Thanks for your comments, I will try to make something up.

@GrahamDumpleton
Copy link
Contributor

For whoever looks at this, one thing you will need to contend with is the decision that $HOME is /opt/app-root/src. IMHO this was a bad design choice for various reasons. Either way, pipenv will want to create stuff under $HOME, so you will want to override that behaviour for a start and have it use a directory under /opt/app-root instead otherwise it will not play nice with local development workflows involving oc cluster up where you mount a local file system directory on top of /opt/app-root/src to allow live code updates.

@GrahamDumpleton
Copy link
Contributor

So $HOME is not an issue as pipenv will use the virtual environment which already exists. You do need to be very careful though. Due to what I would regard as a bug in pipenv, if you run pipenv --two or pipenv --three when pipenv is installed inside of a virtual environment, it will first say, okay I will use the virtual environment. It then though goes on to delete the virtual environment, which for us is /opt/app-root, thus destroying the virtual environment and all the application code under /opt/app-root/src. Once it does that it decides, wait, I didn't create that virtual environment in the first place, so then aborts. Already too late though and it has destroyed everything.

Anyway, for now, the following can be used to handle a Pipfile.

Create executable script .s2i/bin/assemble which includes:

#!/bin/bash

pip install --no-cache-dir powershift-cli[image]

exec powershift image assemble

Create executable script .s2i/bin/run which includes:

#!/bin/bash

exec powershift image run

Create executable script .s2i/action_hooks/pre_build which includes:

#!/bin/bash

pip install --no-cache-dir pipenv

Create executable script .s2i/action_hooks/build which includes:

#!/bin/bash

pipenv install

Yes you could have done it all in custom assemble, albeit would need to add fix-permissions at end of custom assemble as well.

I used the powershift-image package to show benefit of having action hooks once more. ;-)

@GrahamDumpleton
Copy link
Contributor

One issue with this approach will be that pipenv wants to install lots of its own packages. If these get installed into existing virtual environment, there could end up being a clash with versions of packages that the user itself wants, or what may get installed by a requirements.txt file if for some reason that still exists. Having pipenv installed outside of the virtual environment outside of the main virtual environment we use gets a bit messy though. You can't use the per user Python installation area as that may conflict with packages from SCL that pipenv itself may need, and that would have to be done without the main virtual environment being activated or used as can't use per user Python installation area at same time as virtual environment. So this is all going to need a bit more research to come up with solution that avoids issues. For now, the above will most likely work in most cases.

@torsava
Copy link
Member

torsava commented Oct 3, 2017

@GrahamDumpleton Thanks for the extensive rundown! We might want to just recommend using powershift like described, but I'll leave that up to Michal.

As for pipenv dependencies having different versions than the versions developer desires, I believe if you try to pipenv install a different version of a package that pipenv depends on, it will happily comply and install it, so I think that might not be a problem.

@GrahamDumpleton
Copy link
Contributor

The problem with pipenv versions is that because it is using the same virtual environment as pipenv is installed in, if pipenv pulls in a pinned version of a package initially, if a user then has same package name in requirements.txt but doesn't specify a version, it will use whatever version is already installed and will not necessarily pull down the latest as the user might expect. This is the default behaviour of pip which pipenv would use under the covers and this would be what will happen unless pipenv is using -U or --update option to pip to force it to always update to latest version of packages. Will need to look further into pipenv to confirm what it does.

@torsava
Copy link
Member

torsava commented Oct 3, 2017

Ah, I was thinking people using pipenv would most likely have a Pipfile.lock file, but you're right that in other cases it would create an unintuitive behaviour. If that can indeed be mitigated by use of the --update flag, however, then it should be fine.

@mcyprian
Copy link
Collaborator

I've tried a couple of different approaches and pipenv locations. The best solution in my opinion is to install pipenv to per user Python installation area before the main virtualenv is activated, pipenv will have /opt/rh/rh-python36/root/usr/bin/python3 shebang, so activation of virtualenv won't affect it. Pipenv will than use existing virtualenv to install dependencies of the application without recreating it.

  • per user Python installation area is reserved for pipenv and its dependencies
  • main virtualenv is reserved for application dependencies
  • there is no danger of application code loss

In case SCL pip is too old for pipenv (image <= centos/python-35-centos7) newer version will be installed to per user area and will be preferred in sys.path of SCL python binary (/opt/rh/rh-python36/root/usr/bin/python), but per user area is not included sys.path of python binary inside virtualenv (/opt/app-root/bin/python). When virtualenv is activated which pip is /opt/app-root/bin/pip - original version of pip copied from SCL pip, so I don't think this is an issue. My proof of concept implementation is here.

@torsava
Copy link
Member

torsava commented Oct 10, 2017

@mcyprian So if we needed a newer pip for pipenv, we would first create the virtualenv, then update pip outside of it and install pipenv to the per user directory?

I don't see a problem with this approach, so 👍 from me.

@mcyprian
Copy link
Collaborator

@torsava No, newer pip is installed automatically to per user directory when we run pip install --user pipenv in the Dockerfile, installation procedure is always the same, but there is extra pip in user site if pipenv needs it.

@GrahamDumpleton
Copy link
Contributor

GrahamDumpleton commented Oct 10, 2017

Using per user site-packages has the potential for problems. I am not confident that it is something we want to use because of possible conflicts with existing packages installed with SCL. I have blogged about this issue before at:

Anyway, see:

for related discussion of how to bootstrap pipenv so can be used. This is the bug report I created for the problem I found.

@mcyprian
Copy link
Collaborator

mcyprian commented Oct 12, 2017

The only possible conflict here is pip, setuptools or virtualenv and as I mentioned before, newer version of pip is installed to user site in case SCL pip can't be used by pipenv, I don't see any big problem here, but I agree that installing it to the separate venv and symlinking the executable (similar to pipsi approach) is even cleaner solution.

mcyprian added a commit to mcyprian/s2i-python-container that referenced this issue Oct 18, 2017
Pipenv, the higher-level Python packaging can be used to manage
application dependencies on user demand. Pipenv and its dependecies
are sandboxed so they cannot conflict with neither system packages
nor application dependencies.

Resolves  sclorg#214.
@mcyprian mcyprian self-assigned this Nov 8, 2017
@goern
Copy link
Contributor Author

goern commented Nov 21, 2017

Thanks guys!! 👍

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

No branches or pull requests

5 participants