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

Determine name for Recv body type #2971

Closed
Tracked by #2345
seanmonstar opened this issue Aug 30, 2022 · 10 comments · Fixed by #3022
Closed
Tracked by #2345

Determine name for Recv body type #2971

seanmonstar opened this issue Aug 30, 2022 · 10 comments · Fixed by #3022
Labels
A-body Area: body streaming.
Milestone

Comments

@seanmonstar
Copy link
Member

seanmonstar commented Aug 30, 2022

We temporarily renamed the old hyper::Body struct to hyper::body::Recv in #2966. That was to unblock #2839, since we wanted to use the name for the trait instead. But, the name should be properly considered.

The purpose of this type is: an implementation of Body to represent bodies received from a remote. So, the client side would see this in the Response, and the server side would see this in the Request. It is not meant to be constructed by users as a "default" implementation.

Some names I've seen or considered before:

  • Recv
  • RecvStream/RecvBody
  • Streaming
  • Incoming
  • Remote
  • Wire/FromWire

It might help to envision the type signature users will frequently see it as, such as Request<Streaming> (or insert any other name in there).

@seanmonstar seanmonstar added B-rfc Blocked: More comments would be useful in determine next steps. A-body Area: body streaming. labels Aug 30, 2022
@seanmonstar seanmonstar moved this to Needs discussion / design in hyper 1.0 Aug 30, 2022
@seanmonstar seanmonstar added this to the 1.0 RC1 milestone Aug 30, 2022
@seanmonstar
Copy link
Member Author

Pinging because you may have opinions about the name: @hawkw @LucioFranco @davidpdrsn.

@Qard
Copy link

Qard commented Aug 31, 2022

Node.js uses IncomingMessage. Making the connection to Body clearer though would be good. Perhaps IncomingBody?

@davidpdrsn
Copy link
Member

Assuming that the user would never construct these themselves then I like IncomingBody.

@yerke
Copy link

yerke commented Sep 23, 2022

ReqBody?

@seanmonstar
Copy link
Member Author

It's the body of a Request for servers, but it's the body of Responses for the client.

@djc
Copy link
Contributor

djc commented Sep 23, 2022

ReqBody doesn't work well because on the client it would be the RespBody.

I like RecvBody. I think the -ing forms are odd for a 'static type name, and I think it should be a name that includes Body because that's what it's called in the HTTP standard.

@yerke
Copy link

yerke commented Sep 23, 2022

IncomingBody sounds good then.

@hansonchar
Copy link

hansonchar commented Sep 23, 2022

Another similar name/idea would be IngressBody. Is this always an http related body? If so, the name HttpBody may also be considered.

@andresmargalef
Copy link

I think more explícit is better and because is a request and responde body, then i like HttpBody

@tomkarw
Copy link
Contributor

tomkarw commented Sep 25, 2022

I like the idea of hinting that it's a stream, I know for some it's confusing that you don't get the whole headers+body on the same await. So RecvStream, IncomingBody or something like BodyStream/StreamingBody would be my choice.

EDIT: On second thought, RecvStream name is too detached from the concept of HTTP Body.

Repository owner moved this from Needs discussion / design to Done in hyper 1.0 Oct 25, 2022
seanmonstar added a commit that referenced this issue Oct 25, 2022
The concrete "recv stream" body type is renamed to `Incoming`.

Closes #2971
@seanmonstar seanmonstar removed the B-rfc Blocked: More comments would be useful in determine next steps. label Dec 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-body Area: body streaming.
Projects
No open projects
Status: Done
Development

Successfully merging a pull request may close this issue.

8 participants