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

Expand environment variables with default/fallback value in requirements.txt #5421

Closed
pdesgarets opened this issue May 17, 2018 · 5 comments
Closed
Labels
auto-locked Outdated issues that have been locked by automation C: requirement file Using `requirements.txt` state: needs discussion This needs some more discussion type: feature request Request for a new feature

Comments

@pdesgarets
Copy link

pdesgarets commented May 17, 2018

What's the problem this feature will solve?
If I want to use the POSIX variable expansion feature in a requirements.txt file, every developer has to change its habits and export variables in its shell settings. I want existing developers to be able to continue developing on my project without changing anything in their settings.

Describe the solution you'd like
Use POSIX defined variable substitions with default value, ${parameter:-word} or/and ${parameter-word}.

Alternative Solutions
Nothing coming to my mind

Additional context
POSIX reference : http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_06_02

I can help implementing this, but I'd like to get some feedback from maintainers before putting some work on it and submitting a PR :)

@pdesgarets pdesgarets changed the title Feature Request : expand environment variables with default/fallback value Feature Request : expand environment variables with default/fallback value in requirements.txt May 17, 2018
@pfmoore
Copy link
Member

pfmoore commented May 17, 2018

Some initial thoughts. Those substitutions are not familiar to people with a Windows background. Specifically, I've no idea without hunting out references what the difference is between ${parameter:-word} and ${parameter-word} (in fact it took me two readings to even spot that they were different). So while I appreciate that familiarity is important, usability on all platforms is too. At a minimum, any PR would need to fully document the additional syntax - "like POSIX" isn't sufficient IMO (and a pip-specific form may also be worth considering - we do not want to have to deal with queries like "why isn't such-and-such obscure POSIX form supported?").

I'd also be interested in seeing actual use cases. My recollection of the variable expansion feature was that it was motivated by the need to not hard code user credentials in requirements.txt (a use that wouldn't benefit from this feature). I'm not clear what real-world situations this new proposal would enable, and I'm reluctant to simply continue increasing the complexity of the requirements.txt format without good use cases that we know we want to support.

@pradyunsg pradyunsg added state: needs discussion This needs some more discussion type: feature request Request for a new feature C: requirement file Using `requirements.txt` labels May 17, 2018
@pradyunsg pradyunsg changed the title Feature Request : expand environment variables with default/fallback value in requirements.txt Expand environment variables with default/fallback value in requirements.txt May 17, 2018
@pdesgarets
Copy link
Author

Thanks for your feedback.

I totally agree on the "it should be documented in pip doc rather than just saying 'go see POSIX doc'".

Regarding a pip-specific syntax : I get your point, but I think defining a specific syntax would be an unneeded source of issues. For instance, the :/:- difference is an appropriate question that would have emerged maybe much later if I had not "copied" it from POSIX.
"why isn't such-and-such obscure POSIX form supported?" : My typical answer would be "because nobody has submitted a use-case, and no developer has estimated that it was an important-enough issue to spend some time on it. If it does not imply complicated maintainance, please go ahead and submit a PR."
On a question like this one, one must refocus the question from "is it close enough to POSIX syntax ?" to "is there a need for this ?"

Regarding the use case for this: my case is quite specific, I want to use variable expansion to use a different Git remote for some developers. My broader point is "do not change developers habits if it does not add value for them".

@pfmoore
Copy link
Member

pfmoore commented May 17, 2018

Regarding the use case for this: my case is quite specific, I want to use variable expansion to use a different Git remote for some developers

I'd consider that too specialised - the code needed to parse and handle the syntax you're suggesting is not trivial, and while the requirements format is technically pip-specific, other tools do try to parse it, so we would be making things harder for them, too.

My broader point is "do not change developers habits if it does not add value for them".

I don't agree with that. Pip shouldn't cater for every single variation of user behaviour. Users need to work with the tool, not expect it to change to suit them. In this case, why shouldn't the users simply use a standard remote name (or have a script that modifies requirements files to match their needs)?

I'd rather say that pip should focus on the common workflows and make them as smooth as possible. Non-standard workflows should be possible, but we shouldn't over-complicate support (or handling of the normal workflows) to cater for them.

The other pip developers may disagree, of course - this is just my personal view.

@pradyunsg
Copy link
Member

The other pip developers may disagree

I agree. I'm also not convinced that the complexity that is being proposed to be added here is justified by the presented use cases.

@chrahunt
Copy link
Member

chrahunt commented Sep 3, 2019

Based on the discussion above it looks like we will not be implementing this feature (without additional justification). For that reason I will close this issue. Please let us know if there are additional use cases or someone is interested in spending time implementing and reviewing this!

@chrahunt chrahunt closed this as completed Sep 3, 2019
@lock lock bot added the auto-locked Outdated issues that have been locked by automation label Oct 3, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Oct 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
auto-locked Outdated issues that have been locked by automation C: requirement file Using `requirements.txt` state: needs discussion This needs some more discussion type: feature request Request for a new feature
Projects
None yet
Development

No branches or pull requests

4 participants