-
Notifications
You must be signed in to change notification settings - Fork 106
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
Add udp #327
base: main
Are you sure you want to change the base?
Add udp #327
Conversation
self._stream.close() | ||
# TODO: Correct way to destroy all handlers? | ||
for task in self._running_handlers: | ||
task.cancel() |
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.
The proper pattern here according to the asyncio docs is:
task.cancel()
try:
await task
except asyncio.CancelledError:
pass
This gives the loop the opportunity to inject the cancellation exception into the task so that it doesn't end up being reported as an unretrieved exception.
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.
Thanks! I missed it. And probably there are more bugs, I wrote it without much revising.
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 would be nice to hold off on merging this until we resolve the timeline for trio. If it's to be soon, we should keep this as a reference and re-implement using trio, otherwise we can proceed if conversion to trio is likely to happen on a longer timeline.
This is still just a "prototype" of what could be a UDP implementation. I just made the pull request for being easily referable from the issue. |
Sorry for closing @pipermerriam I was browsing on the phone, and I hit "Close and comment" accidentally. |
If you would like to work on that I'd be happy to help in any way I can. That could be reviewing pull requests, funding a small grant, etc. |
We should first check how asyncio compatibility would work, because most people use asyncio. Or which limitations/difficulties may arise. However, I think, Trio's sockets API (apart from Nursery) are much better than asyncio's. And even more for getting a Unified Interface for TCP and UDP. |
The plan would be to drop compatibility with |
Removed 'drain' from Stream unified interface Linted Linted Fixed little ineffiencies, and added bit of documentation Fixed ServerStream closing With previous implementation, when you closed the stream, you closed the stream shared by all UDP handlers from the same Server Fixed inconsistent method resolution in UDP Streams Linted
some useful links for getting this ported to trio: |
This is the code related to UDP implementation, where it is trying to Resolve #326 .