Version 0.28. #3370
Merged
Version 0.28. #3370
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
T-256
approved these changes
Oct 28, 2024
httpx/_api.py
Outdated
Comment on lines
87
to
94
| * **verify** - *(optional)* SSL certificates (a.k.a CA bundle) used to | ||
| verify the identity of requested hosts. Either `True` (default CA bundle), | ||
| a path to an SSL certificate file, an `ssl.SSLContext`, or `False` | ||
| (which will disable verification). | ||
| * **cert** - *(optional)* An SSL certificate used by the requested host | ||
| to authenticate the client. Either a path to an SSL certificate file, or | ||
| two-tuple of (certificate file, key file), or a three-tuple of (certificate | ||
| file, key file, password). |
There was a problem hiding this comment.
Perhaps it's better to keep these with *(deprecated)* tag?
There was a problem hiding this comment.
I'm neither for or against that.
httpx/_transports/default.py
Outdated
Comment on lines
142
to
146
| if verify is not None or cert is not None: # pragma: nocover | ||
| # Deprecated... | ||
| ssl_context = create_ssl_context(verify, cert) | ||
| else: | ||
| ssl_context = ssl_context or SSLContext() |
There was a problem hiding this comment.
perhaps, raising error when used together?
httpx.get("https://example.com/", verify=False, ssl_context=httpx..SSLContext())There was a problem hiding this comment.
Okay yep... I'm going to retitle our version-1.0 branch and will resolve there.
Co-authored-by: T-256 <132141463+T-256@users.noreply.github.com>
Co-authored-by: T-256 <132141463+T-256@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Aiming to get our current 1.0 work merged into mainline.
This PR would be a more gentle upgrade, since
verifyandcertbecome deprecated but do continue to function with warnings. I'd value other maintainers perspective on this.The one other aspect to this worth considering is that it would be a really small change from here for us to switch to
truststoreinstead ofcertifi. If we do want to make that switch, now might be a good time to do so?