More general interface for UdpFramed
#5969
Open
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
GoldsteinE
force-pushed
the
generalize-udp-framed
branch
3 times, most recently
from
5b3f0b0
to
c6e7472
Compare
Previously `UdpFramed` only accepted smart pointers to `UdpSocket`. This commit changes it to accept anything that has `.poll_recv_from()` and `.poll_send_to()`. It allows for easier testing and more composability.
GoldsteinE
force-pushed
the
generalize-udp-framed
branch
from
c6e7472
to
e2d527f
Compare
Darksonn
reviewed
Sep 2, 2023
Comment on lines
+28
to
+36
| impl<T: Borrow<UdpSocket>> PollRecvFrom for T { | ||
| fn poll_recv_from( | ||
| &mut self, | ||
| cx: &mut Context<'_>, | ||
| buf: &mut ReadBuf<'_>, | ||
| ) -> Poll<io::Result<SocketAddr>> { | ||
| (*self).borrow().poll_recv_from(cx, buf) | ||
| } | ||
| } |
There was a problem hiding this comment.
Instead of doing this, maybe this should just be added to the list for the next breaking tokio-util release.
There was a problem hiding this comment.
There’s also an option to do both: merge this now and remove the blanket impl on the next breaking release.
Darksonn
added
the
S-breaking-change
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.
Previously
UdpFramedonly accepted smart pointers toUdpSocket. This commit changes it to accept anything that has.poll_recv_from()and.poll_send_to(). It allows for easier testing and more composability.Motivation
My specific usecase is the following: I have a wrapper for
UdpSocketthat records some low-level metrics. It’s impossible to do so on a higher level (i.e. wrappingUdpFramed), because it requires basically replacing.poll_recv()with different syscalls. I still want to re-use framing logic, but I need to pass a different type to it.UdpFramedalso doesn’t allow to pass a mock implementation for testing.Solution
I added new traits for sending / receiving datagrams and relaxed the requirements of
UdpFramedto make it use these traits.Design notes
udp/, because the interface is really specific. Unix sockets have similar APIs, but use paths instead of socket addrs.&mut selfinstead of&selfto make external implementations easier.