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

Allow passing options of commands via environment variables #2537

Closed
wants to merge 2 commits into from

Conversation

fridex
Copy link
Contributor

@fridex fridex commented Jul 7, 2018

We would like to provide a way to automatically submit options during builds via environment variables. This is not a standard way how to enable click's auto_envvar_prefix, but it works with console_scripts entrypoint.

@fridex
Copy link
Contributor Author

fridex commented Jul 7, 2018

Worth to document such change; also the pipenv --envs output will probably need to be adjusted to provide some info.

@uranusjr
Copy link
Member

uranusjr commented Jul 7, 2018

This would be a very nice addition, but unfortunately it can’t be done this simple :( We have environment variables that are not in line with option names, e.g. PIPENV_PYTHON is totally not related to --python. We will need to grep-replace-test the whole code base to ensure this won’t happen. Probably even need to find a way to prevent this from accidentally happening in the future.

@fridex
Copy link
Contributor Author

fridex commented Jul 8, 2018

Setting the auto_envvar_prefix this way causes availability of environment variables only in commands and not CLI itself (click's context is already instantiated for cli when parsing). But I see PIPENV_PYTHON being used explicitly from env for commands so for example PIPENV_INSTALL_PYTHON will clash with PIPENV_PYTHON. Is that what you are referring to (priorities for applying options)?

I quickly tried to go through the listing of environment variables and didn't spotted any other collision .

BTW listing in docs does not state PIPENV_PYTHON.

@uranusjr
Copy link
Member

uranusjr commented Jul 8, 2018

Yes it is. I didn’t actually check what environment variables we are currently using, but PIPENV_PYTHON came to me immediately as a naming conflict when seeing this PR. It is an internal environment variable used to communicate what Python is currently being used between Pipenv processes, so it likely can be safely renamed. A simple find-and-replace inside the repository should be enough.

@fridex
Copy link
Contributor Author

fridex commented Jul 8, 2018

so it likely can be safely renamed

IMHO I would keep it for backwards compatibility. Based on click's logic

$ export PIPENV_PYTHON=/usr/bin/python2.7
$ pipenv install requests --python /usr/bin/python3.6

Is equivalent to:

$ export PIPENV_PYTHON=/usr/bin/python2.7
$ export PIPENV_INSTALL_PYTHON=/usr/bin/python3.6
$ pipenv install requests

So this PR should not introduce any new logic for that.

@uranusjr
Copy link
Member

uranusjr commented Jul 8, 2018

Oh! I didn’t realise that, good to know! Indeed this likely wouldn’t cause any problems if that is the case. I will find time to review if there are any side effects. Thanks for the clarification 😃

@kennethreitz
Copy link
Contributor

Please use the new PEEP process: https://github.com/pypa/pipenv/blob/master/peeps/PEEP-000.md

@fridex
Copy link
Contributor Author

fridex commented Sep 2, 2018

@kennethreitz where can I find steps on how to submit a PEEP?

@kennethreitz
Copy link
Contributor

kennethreitz commented Sep 2, 2018 via email

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 this pull request may close these issues.

4 participants