Support Pipfile #168
Support Pipfile #168
Conversation
Fixes #151.
|
It looks like this is forcing python 3.6 if you want to use Pipenv. Is there some restriction on using that version of Python when using Pipenv? |
|
@brycekahle Ah sorry, I should have mentioned that. Python version set in You can see complete example at https://docs.pipenv.org/basics/#specifying-versions-of-python |
|
If there is no |
|
Yes, it defaults to |
|
Currently we have 2.7 as the default, so I think we should maintain that for Pipenv. |
| if $HOME/python$PIPENV_RUNTIME/bin/pipenv install | ||
| then | ||
| echo "Pipenv dependencies installed" | ||
| if source $($HOME/python$PIPENV_RUNTIME/bin/pipenv --venv)/bin/activate |
There was a problem hiding this comment.
fwiw, doing this instead of pipenv shell and pipenv run means .env loading is not supported, which could break some Pipfile if it is expecting variables from .env to be present.
https://github.com/pypa/pipenv/blob/master/docs/advanced.rst#-automatic-loading-of-env
https://github.com/pypa/pipenv/blob/master/docs/advanced.rst#-support-for-environment-variables
Note that it isnt even possible to use .env vars during pipenv install. see pypa/pipenv#1906 (comment)
As a result, probably a good idea to set PIPENV_DONT_LOAD_ENV and document that .env isnt used by Netlify.
There was a problem hiding this comment.
Thanks for the feedback! It seems to me that ENV variables are meant to be handled some other way on production. E.g. using https://www.netlify.com/docs/continuous-deployment/#build-environment-variables.
Based on discussion in pypa/pipenv#773 (comment) pipenv shell is meant only for development and using source .../bin/activate is OK for production.
run-build-functions.sh
Outdated
| echo "Python version set to $(python -V)" | ||
| else | ||
| echo "Error activating Pipenv environment" | ||
| echo "Please see https://github.com/netlify/build-image/#included-software for current versions" |
There was a problem hiding this comment.
this isnt particularly useful here, as they would go to that page to learn that "Latest version." is the current version, and they would find the current version was actually in the log a few lines above this. ;-)
There was a problem hiding this comment.
Well, I thought this refers to list of python versions user can use.
| elif [ -f Pipfile ] | ||
| then | ||
| echo "Installing dependencies from Pipfile" | ||
| if $HOME/python$PIPENV_RUNTIME/bin/pipenv install |
There was a problem hiding this comment.
might be useful to set PIPENV_VENV_IN_PROJECT as multiple venvs are not needed on Netlify builds. setting that might improve performance a little.
There was a problem hiding this comment.
I think we should move this (and the export lines from above) to the Dockerfile if possible. I'd rather pipenv be installed already, rather than done every build on-demand.
This way we use virtualenv directly without going through Python Env Wrapper.
|
ping @brycekahle |
|
@jayvdb Sorry for the delay. I need some time to look at this more in depth. |
run-build-functions.sh
Outdated
| @@ -14,6 +14,11 @@ export GIMME_CGO_ENABLED=true | |||
| export NVM_DIR="$HOME/.nvm" | |||
| export RVM_DIR="$HOME/.rvm" | |||
|
|
|||
| # Pipenv configuration | |||
| export PIPENV_RUNTIME=3.6 | |||
There was a problem hiding this comment.
I think I would prefer this to be 2.7 if possible, since that is our default python version at the moment.
| elif [ -f Pipfile ] | ||
| then | ||
| echo "Installing dependencies from Pipfile" | ||
| if $HOME/python$PIPENV_RUNTIME/bin/pipenv install |
There was a problem hiding this comment.
I think we should move this (and the export lines from above) to the Dockerfile if possible. I'd rather pipenv be installed already, rather than done every build on-demand.
|
@brycekahle I'm on vacation so I'll look into it as soon as I can :) |
So it goes in line with default Python version.
|
@brycekahle Thanks for the feedback! I made the changes you requested. |
As mentioned in #151 python community is heavily adopting dependency manager Pipenv. It's even recommended in official guide Managing Application Dependencies. It combines virtualenv creation, runtime specification, dependencies and their locked versions into two files.
I added support for
Pipfile. The script first checks forruntime.txtand then forPipfile.You can test the implementation on https://github.com/Liduska/pipenv-build-example, which is copy of repository I use with
requirements.txtandruntime.txtremoved. You can find original repository on https://github.com/Liduska/coffeeshop-netlify.Thanks for the great product!