-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Move ssl-related params from TCPConnector to request args #1128
Comments
I think they should not be moved to single requests but to ClientSession instead. Multiple requests to the same host+port can reuse connection (or also not reuse, it's statically unknown in general) — Line 340 in 3da3a01
@asvetlov do you agree? |
I think if |
It can, of course, but I'm not sure if there are real advantages of doing this. But on second thought I see that it's probably fine for user, it would be just a little harder to implement. So, okay… (at first I thought that there would be also things like which versions of TLS and which ciphers to negotiate with the server, and then there's no meaningful way of changing these constraints for next request with the same connection in general way, other than closing the connection and doing the handshake again, but if this level of control is not going to be allowed, then setting just |
Fixed by #2184 |
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a [new issue] for related bugs. |
Long story short
Now ssl configuration is tightly coupled with
TCPConnector
.It is not convenient (user should create a connector and pass it into session) and disallows to use the same
ClientSession
for working with different ssl contexts etc.I think we should move
verify_ssl
,fingerprint
andssl_context
intoClientSession.request
andClientSession.ws_connect
like we have done forproxy
andproxy_auth
(see #998).Current TCPConnector's parameters should be kept as default values which may be overridden by request without any warning/error.
The text was updated successfully, but these errors were encountered: