Skip to content
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

Better names for h3::quic #146

Open
eagr opened this issue Nov 16, 2022 · 2 comments
Open

Better names for h3::quic #146

eagr opened this issue Nov 16, 2022 · 2 comments

Comments

@eagr
Copy link
Member

eagr commented Nov 16, 2022

The name poll_accept_recv is a bit counter-intuitive. Is it accepting the peer's request for receiving? But it's actually meant for accepting a unidirectional (send) stream from the peer, or in the caller's perspective, opening a stream for receiving. So it may not be a good name for the case, as it requires people to do a bit interpretation to figure out what the function is supposed to do.

quinn's API Connection::accept_uni feels much more intuitive, accepting unidirectional communication from the peer. And it's clear that it's for receiving in the context.

Proposal for API changes:

  • poll_accept_recv() -> poll_accept_uni()
  • poll_open_send() -> poll_open_uni() (to be symmetric and predictable)
  • Maybe?
    • poll_accept_bidi() -> poll_accept_bi() (as poll_accept_bi is more predictable when we have poll_accept_uni)
    • poll_open_bidi() -> poll_open_bi() (to be symmetric)
@camshaft
Copy link
Contributor

I actually think the current names make more sense, TBH. Unidirectional is underspecified, in that it's missing information about the direction of the data. If I have a unidirectional stream, what operations can I perform on it? In order to know that, you need to know the owner of the steam.

In contrast, if I have a send stream, it means the local connection is able to send on it. Similarly, a recv stream means the local connection is able to receive on it. It also means bidirectional streams can be naturally split and it's clear what side can do what operations.

@eagr
Copy link
Member Author

eagr commented Nov 17, 2022

I totally agree that names like SendStream and RecvStream should stay. I'm just proposing to change some of the function names.

Let me rephrase this a bit. I think the the name poll_accept_recv kinda contradicts with what happens. The client initiates a stream (only for sending) to the server and the server accepts it. If we stick to using the word "accept", technically the name should probably be poll_accept_send(_stream). In that sense, putting "accept" and "recv" together may not be a good idea.

Coming back to what we really want to distinguish the functions for

  • accept incoming bidirectional stream
  • accept incoming unidirectional stream (the direction is quite clear in the context of h3)

These translate directly to accept_bi and accept_uni. And the combination is more symmetric and intuitive.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants