-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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 parsing of verify_ssl=false in urls with authentication in Pipfile #2656
Conversation
Can someone post what the buildkite says is failing? |
Sorry I’m out of town due to a family emergency and only me and Kenneth have accesss. If you remind me in a day I can check for you or you can run the build on a Linux vm if you didn’t already to find out. I can tell you that it builds python 2.7 first and that’s likely what failed |
@techalchemy are you able to post the error output now? |
Not from Buildkite, but these are the errors I got
Looks like you missed some quotes in your test fixture. |
1cb191c
to
7f72b94
Compare
@uranusjr Thanks, fixed. |
7f72b94
to
f78637f
Compare
Thanks for the contribution! I don't really have any concerns here, looks pretty good to me |
Signed-off-by: Dan Ryan <dan@danryan.co>
f78637f
to
fd5b4ba
Compare
@@ -0,0 +1 @@ | |||
Fixed a bug which sometimes caused pipenv to parse the ``trusted_host`` argument to pip incorrectly when parsing source URLs which specify ``verify_ssl = false``. |
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.
This bug wasn't a result of verify_ssl
being false
it was because the source contained a username and password.
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.
Feel free to rewrite the fragment if you feel it is incorrect!
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.
I mean, while the bug may not be caused by verify_ssl
being false
(I didn't say that it was caused by setting verify_ssl = false
, only that it sometimes occurred when parsing URLs which specified this option), it can certainly only apply to URLs that specify verify_ssl = false
as I mention in the news fragment, wouldn't you say?
Fixed a bug which sometimes caused pipenv to parse the
trusted_host
argument to pip incorrectly
This is the nature of the bug, no?
parsing source URLs which specify
verify_ssl = false
.
This is a condition under which the bug occurs, right? I didn't want to get super detailed, as it's most likely an edge case that most users won't encounter -- there is a bug parsing the trusted host argument sometimes if you specify the argument via your pipfile. There are other ways to specify trusted hosts, but this is the only one that is handled by pipenv directly, so it seems relevant to caveat this by highlighting the specific context without delving into the low level, 'if you first type x, then y, then z, then put a thing here, etc'.
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.
Sorry, I got totally confused about the nature of the bug, after taking a look at the code again and remembering what the bug is. I totally agree with what you wrote.
pipenv
breaks when the first specified source in the Pipfile contains a username and password.