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

pdm export support env variables in credentials #1939

Closed
Drachenfels opened this issue May 19, 2023 · 6 comments · Fixed by j178/pdm#1
Closed

pdm export support env variables in credentials #1939

Drachenfels opened this issue May 19, 2023 · 6 comments · Fixed by j178/pdm#1
Labels
⭐ enhancement Improvements for existing features

Comments

@Drachenfels
Copy link

Is your feature request related to a problem? Please describe.

Pipelines sometimes work with generated requirements.txt as backward compatibility, if one uses a different index like this:

[[tool.pdm.source]]
url = "https://${PDM_USERNAME}:${PDM_PASSWORD}@path/to/custom/pypi/package/simple/"
verify_ssl = true
name = "pypi"

and PDM_USERNAME (example: john) / PDM_PASSWORD (example: password123) is set,

and runs pdm export like this:

pdm export --prod -o requirements.txt --without-hashes

the resulting requirements file will have a username and password in plaintext:

# This file is @generated by PDM.
# Please do not edit it manually.

zipp==3.15.0
--index-url https://john:password123@path/to/custom/pypi/package/simple/

Describe the solution you'd like

PIP allows to use env variables that are expanded at runtime: https://pip.pypa.io/en/stable/reference/requirements-file-format/#using-environment-variables

My proposal would be to add a switch --no-env-expansion (please suggest a better name) that will generate an export file but with an env notation that pip likes, example:

# This file is @generated by PDM.
# Please do not edit it manually.

zipp==3.15.0
--index-url https://${PDM_USERNAME}:${PDM_PASSWORD}@path/to/custom/pypi/package/simple/
@Drachenfels Drachenfels added the ⭐ enhancement Improvements for existing features label May 19, 2023
frostming added a commit that referenced this issue May 22, 2023
Fixes #1939

Signed-off-by: Frost Ming <me@frostming.com>
@Drachenfels
Copy link
Author

Hi @frostming

I wonder what was the resolution of this issue, if I read the commit correctly, you consider it was actually a bug and from now on by default, we are going to have env vars in requirements. Am I correct?

@frostming
Copy link
Collaborator

Yes, I think preserving the env vars make more sense. And the variable substitution is also supported by pip.

It may be a little breaking change but I think people writing variables in their source settings can expect it.

@berislavlopac
Copy link

Wouldn't it make sense to keep a runtime option to force the variable expansion?

@berislavlopac
Copy link

@frostming you said:

And the variable substitution is also supported by pip.

Can you point out this in any documentation? As far as I can tell, pypa/pip#4789 is still open... 🤔

@frostming
Copy link
Collaborator

frostming commented Jun 9, 2023

@berislavlopac You are confusing two different requirements. pip has always supported using environment variables in requirementst.txt(since 10.0): https://pip.pypa.io/en/stable/reference/requirements-file-format/#using-environment-variables

While the issue you linked to is asking for env expansion in pip.conf. This issue also talks about the requirements.txt exported by PDM.

@berislavlopac
Copy link

Thank you @frostming. How about this question?

Wouldn't it make sense to keep a runtime option to force the variable expansion?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⭐ enhancement Improvements for existing features
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants