Add environment variable expansion in config files

[kofron]

I've added a helper function that allows for environment variable expansion in config files. The syntax is supposed to be as natural as possible - "${MY_CONFIG_VAR}" does what you'd expect.

In the event that an environment variable is not defined, the behavior is to simply ignore it and leave it as-is.

Also, it looks like because I branched from master that I've pulled in some documentation changes from there with that PR. If that's an issue I can re-create this branch off of develop and cherry-pick into it.

[kofron]

@sdispater would love feedback!

@loop0 here's an implementation of what I mentioned in that issue.

[funkyfuture]

i like the idea and take the liberty to point at Docker-Compose's default and required capabilities as possible extension.

[kofron]

thanks @funkyfuture. I'll take a look at that as well

[loop0]

If you override the TomFile.read() and apply the variable expansion there I think it would be better and could avoid future bugs where we call the read() without remembering to call expand_environment_vars() after.

[rooterkyberian]

I will like to start with stating that as this PR stands right now this is equivalent to

envsubst < pyproject.template.toml > pyproject.toml

which IMO is cleaner and less a headache to support. Just don't forget to add pyproject.toml to .gitignore.
This alone makes me think it is not worth integrating it in poetry, but I guess you have @sdispater to convenience not some random dude:P

The example you gave makes me really dislike this feature

[tool.poetry.dependencies]
my_package = "${MY_PACKAGE_VERSION}"

this makes locking impossible, so throws one of the main features of poetry out of the window - guess there is more to poetry, but still I wonder what other hacks this will bring.
BTW Are you sure you don't want to do this stuff in tox and just specify minimal package version in poetry?

More importantly the silent ignore without having to provide an explicit default? yuck!

IMHO the most sensible example is

[[tool.poetry.source]]
name = "a repo with a secret access key"
url = "https://secret.com/${SUPER_SECRET_KEY}/pypi"

which with silent ignore would be just confusing - instead of error "SUPER_SECRET_KEY was not defined" I would get a 404 error in the best case.

[loop0]

I agree that leaving as is in case the env var is not found is not a good solution, a better way IMO would be to throw an error to the user.

On the template subject: using a template and envsubst makes it hard to maintain, every time you would add a new dependency using poetry cli you would have to also update the template by hand.

My initial proposal was to solve the secret repository problem you also brought up.

Having the ability to use env vars inside pyproject.toml is a very useful feature.

I think we just have to discuss a little bit more to have a mature view of what needs to be done.

[kofron]

You're right @rooterkyberian , the package-version-as-environmental-variable is not a good example use case. However, storing secret keys or repo locations is definitely a use case worth supporting.

I also think using envsubst and a template file is not sufficient - it complicates developer workflows as well as CI/CD. One of the great aspects of poetry is that everything you need to know is contained in pyproject.toml - spreading information out to multiple files is an anti-goal of this feature.

It's certainly a reasonable choice to not silently ignore non-existent environment variables, we could choose to raise on them. The only reason I didn't go that route was the (admittedly probably rare) case in which somebody deliberately had an environment-variable like string contained in a pyproject.toml that they did not intend to expand. If you have ideas about how to pass in sensible defaults, share them!

[rooterkyberian]

@kofron figuring out how to pass defaults is simple (for example use shell like syntax ${env:-default} or ${env:=default}; or something not shell compatible ${env:default}) , but do we have real use case for it?

docker compose does not support defaults, as it overlaps with .env support: docker/compose#3505
.env is supported already by pipenv, maybe poetry should follow suit?
Anyway, at this point this is just argument for delaying default value implementation, while enforcing erroring on missing variable value

[rooterkyberian]

I have just noticed that pipenv does implement the environment variable expansion in config https://github.com/pypa/pipenv/blob/master/docs/advanced.rst#-injecting-credentials-into-pipfiles-via-environment-variables
implementation and some of the discussion: pypa/pipenv#1769

To me it seems quite ugly, as what they are doing is running os.path.expandvars which will leave '${test}' as it is if no test env variable exists. This sounds rather useless compared to throwing error about undefined env variables. The argument seems to be that $ is allowed character in URLs - but I wonder when was the last time anyone saw one of these that couldn't be very well URL encoded to %24 without breaking the URL.

Anyway, like I argued earlier - looking how other projects do this maybe a good idea as it will make feature easier to grasp for the users.

[sdispater]

Thanks for putting this PR together but this is an overhead that I don't want to had to the project.

[robertlagrant]

We also have the case of using a custom index. Currently we just put --extra-index-url=${CUSTOM_INDEX_URL} at the top of requirements.txt and install from that. If this isn't the way Poetry wants to implement that capability, is there an alternative way to avoid hardcoding a secret URL?

[kofron]

We also have the case of using a custom index. Currently we just put --extra-index-url=${CUSTOM_INDEX_URL} at the top of requirements.txt and install from that. If this isn't the way Poetry wants to implement that capability, is there an alternative way to avoid hardcoding a secret URL?

+1 - this was inspiration for this PR, so definitely want to hear what the suggested solution is for this. At present I'm not able to use poetry at work (although I'd love to) for this exact reason.

[metrofun]

Same here, can't migrate to Poetry from Pipenv at work :|

[justinTM]

@sdispater do you have possibly new thoughts on this?

i am trying to use poetry to manage dependencies which are under development, and would like to install on, for example, a remote cluster. I don't want to set up SSH key for cluster dynamically, and I don't want to commit tokens into the repo. So I was hoping to just have my dependencies as git repos instead of building whl and uploading to cluster ever time there is a change

[neersighted]

@justinTM, what you want is easily possible if you switch to the https transport for git. You don't even have to change anything in your project if you use insteadOf in your .gitconfig.

You can then configure auth using poetry config repositories.<name> <git url prefix> and poetry config http-basic.<name> <username> <password>.

Poetry's project format is declarative -- adding templating significantly reduces the interoperability and stability of a Poetry-packaged project and this is unlikely to happen in the main project.

For users spelunking for related feature requests, see #1632 for replacing PyPI's URL using a out-of-repo mechanism, and #5958 for adding additional sources using an out-of-tree mechanism.

Also, please refrain from pinging individual contributors for attention -- people who are interested will see regardless of pings, so it just adds needless targeting of an individual.

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.