-
-
Notifications
You must be signed in to change notification settings - Fork 615
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
Fix leaked URLs with credentials in the output #1146
Fix leaked URLs with credentials in the output #1146
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1146 +/- ##
=======================================
Coverage 99.48% 99.49%
=======================================
Files 36 36
Lines 2736 2752 +16
Branches 324 326 +2
=======================================
+ Hits 2722 2738 +16
Misses 8 8
Partials 6 6
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work! Approved 👍
If you think it's appropriate, I would suggest renaming default_index_url
in compile.py
to better reflect that it's not the real default URL (redacted and used only for the help string).
Yeah, naming is hard :) For example:
What's your suggestion? I'm also tempted to move some logic to a function to not waste global scope: def _get_pip_default(option_name):
install_command = create_command("install")
pip_defaults = install_command.parser.get_default_values()
return getattr(pip_defaults, option_name)
...
@click.option(
"-i",
"--index-url",
help="Change index URL (defaults to {})".format(
redact_auth_from_url(
_get_pip_default("index_url")
)
),
envvar="PIP_INDEX_URL",
) What do you think? |
Moving that into the function looks good. Otherwise, I'd say |
447cc55
to
5430aae
Compare
Move some logic to a function to not waste the global scope. Co-Authored-By: Andy Kluger <[email protected]>
5430aae
to
302d46b
Compare
Addressed in 302d46b. |
Thanks @AndydeCleyre for reviewing this! |
Fixes #1137.
Refs #811 and #1028.
Changelog-friendly one-liner: Fix leaked URLs with credentials in
pip-compile
's output.Contributor checklist