-
-
Notifications
You must be signed in to change notification settings - Fork 377
Bump dependencies from commit 79bdcf #3277
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
Conversation
Pull request was closed
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3277 +/- ##
===============================================
Coverage 100.00000% 100.00000%
===============================================
Files 127 127
Lines 19257 19269 +12
Branches 1301 1300 -1
===============================================
+ Hits 19257 19269 +12
🚀 New features to boost your workflow:
|
I think we're hitting deprecation warnings because we never switch out the underlying objects, instead mutating the SSL context. This change should also be nice if we ever parallelize the tests
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
src/trio/_tests/test_dtls.py
Outdated
| with endpoint(ipv6=ipv6) as client_endpoint: | ||
| client = client_endpoint.connect(address, client_ctx) | ||
| # TODO: why is making a new client context necessary here? | ||
| client = client_endpoint.connect(address, client_ctx_fn()) |
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'm not sure why this was failing without a unique client_ctx (in fact this may still fail, CI run isn't over yet)
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.
It's failing anyways. I don't get this!
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.
OK I think I figured it out: every time we get a client hello, we set the relevant options on the server's SSL context. We shouldn't do that, and now it's pushed up to server creation. Hopefully that works, I don't really know DTLS...
|
This is a big PR but it's mainly just a bunch of changes s.t. DTLS doesn't need to mutate SSL contexts after creating a connection with them (specific changes: making new contexts for each tests and pushing the SSL context mutation for servers to before it gets any connections) and then a few changes due to new mypy version. |
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.
Looks good I think, dtls test changes are a bit more complex but glancing through it I believe that makes sense, though if someone is more familiar with dtls that would be great.
|
DTLS probably works and I don't think we have any experts on it who are going to review this code. If there's any issues I'll make a follow up release whenever this gets released. |
No description provided.