-
-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
Use TLS settings in selecting connection pool #6655
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2828,6 +2828,13 @@ def test_status_code_425(self): | |
assert r5 == 425 | ||
assert r6 == 425 | ||
|
||
def test_different_connection_pool_for_tls_settings(self): | ||
s = requests.Session() | ||
r1 = s.get("https://invalid.badssl.com", verify=False) | ||
assert r1.status_code == 421 | ||
with pytest.raises(requests.exceptions.SSLError): | ||
s.get("https://invalid.badssl.com") | ||
Comment on lines
+2831
to
+2836
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There may not be a better way to test this but I don't know if we have other tests that require contacting a live site with TLS disabled. That may have some durability issues and means we're going to take the first response we get back. Probably minor, but figured I'd call it out. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are many alternatives here, but those are all significantly more effort and this shows the behaviour is fixed before and after handily. I'm sure Linux folks will get pissed but I'm not as bothered about finding time later to do this a different way after we have fixed this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll try to prioritize better (offline) tests soon |
||
|
||
|
||
def test_json_decode_errors_are_serializable_deserializable(): | ||
json_decode_error = requests.exceptions.JSONDecodeError( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,7 +7,7 @@ extras = | |
security | ||
socks | ||
commands = | ||
pytest tests | ||
pytest {posargs:tests} | ||
|
||
[testenv:default] | ||
|
||
|
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.
Not against this, but I think this is the first time we're introducing typing into Requests. I'm curious if we want to start that or push it into typeshed since this will be precedent for future inline typing?
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 is for a private method (that I fully anticipate people abusing) but we're not advertising things are typed and so it's not something I'm concerned with.