Allow usage of Environment Variables inside TOML values

[dvf]

We're relying on os.path.expandvars() to inject environment variables in values.

• Please see Issue https://github.com/pypa/pipenv/issues/1688 for context
• This is the source for PR https://github.com/pypa/pipenv/pull/1769

Some thoughts:

• The Pipfile hash should be determined after values have attempted to be inserted.
• Pipfile shouldn't be intelligent by trying to differentiate env vars from regular characters—if an env var can't be inserted, the string should remain intact.
• As it stands, Pipenv's vendor folder contains some version of this repository. I don't fully understand the reasons behind this, couldn't we pin it as a requirement instead? We could then separate tests.

[jtratner]

1. Can you add notes to the docs about the environment variable parsing that explain how they should be interpreted?
2. I guess there aren't other tests for pipfile right now, but might be nicer / clearer to just write out full example pipfiles. I get the desire to DRY up the tests cases, but it'd be easier to understand what you mean if you just saw in code:

"""
[[source]]
url = "https://$FOO:$PASSWORD@mypypi.whatever.org"

[packages]

requests = "*"
# presumably some git egg style thing where you're putting env vars in git URL
"""

and then (because you have two env vars) you can check what happens when env vars can and cannot be expanded. + that gives you easy route to check hashing behavior :)

[pradyunsg]

I suggest doing something like pypa/pip#3782.

Also these tests will fail on Windows.

[pradyunsg]

I meant to link to pypa/pip#3728.

[dvf]

Thanks for the feedback folks!

@pradyunsg: Can you give some context about why you think these tests will fail on windows? @jtratner: Where would you like these notes to go?

[jtratner]

README.rst seems to have all the docs, so good to put a note there :) On Mon, Mar 19, 2018 at 3:46 PM Daniel van Flymen notifications@github.com wrote:

Thanks for the feedback folks!

@pradyunsg https://github.com/pradyunsg: Can you give some context about why you think these tests will fail on windows? @jtratner https://github.com/jtratner: Where would you like these notes to go?

— You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub https://github.com/pypa/pipfile/pull/105#issuecomment-374410325, or mute the thread https://github.com/notifications/unsubscribe-auth/ABhjqw-fqw3RMnhfyxjcXrhRvLf_jN5Kks5tgDUtgaJpZM4SvAL7 .

[kennethreitz]

Merging — please add another PR with docs updates ASAP.

[gnagel]

Nice! 👍 @dvf

[pradyunsg]

Can you give some context about why you think these tests will fail on windows?

My bad. Not fail.

The point I wanted to make was os.path.expandvars expands %VAR% format variables along with ${var} and $var format ones on Windows, not other OSes. This is an inconsistency across OSes. The approach in the PR I linked above is consistent across OSes -- only expanding ${var} on all OSes.

[dvf]

I see your point but I think this is ok—it’s a Python feature and not application-specific. We shouldn’t deviate from the std lib implementation without good reason.

On Apr 10, 2018, at 6:22 AM, Pradyun Gedam notifications@github.com wrote:

Can you give some context about why you think these tests will fail on windows?

My bad. Not fail.

The point I wanted to make was os.path.expandvars expands %VAR% format variables along with ${var} and $var format ones on Windows, not other OSes. This is an inconsistency across OSes. The approach in the PR I linked above is consistent across OSes -- only expanding ${var} on all OSes.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.