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

Feature: Implement the Body trait for more native data types to improve ergonomics #3746

Open
akneni opened this issue Aug 27, 2024 · 7 comments
Labels
B-rfc Blocked: More comments would be useful in determine next steps. C-feature Category: feature. This is adding a new feature.

Comments

@akneni
Copy link

akneni commented Aug 27, 2024

Currently, the 'String' is the only native data type that implements hyper::body::Body.
If we're passing other data types as a body, we need to add the http_body_util crate and add a little bit of boiler plate when building our request.

use http_body_util::Full;

let some_bytes = vec![0_u8, 1, 2];
let mut req = Request::builder()
    .method("POST")
    .body(Full::<Bytes>::from(some_bytes))
    .unwrap();

Note: While the request builder itself may not require the body to implement the Body trait, both the sender and connection returned by http1::handshake require the request's body to implement it.

With the body trait implemented for Vec<u8> (just one example that probably should implement body) we could leave out the http_body_util crate. Building a request would also become more intuitive and ergonomic.

    let some_bytes = vec![0_u8, 1, 2];
    let mut req = Request::builder()
        .method("POST")
        .body(some_bytes)
        .unwrap();
@akneni akneni added the C-feature Category: feature. This is adding a new feature. label Aug 27, 2024
@zaira-bibi
Copy link

Hi! I would like to try my hand on this issue.

@seanmonstar
Copy link
Member

I've been personally torn on this. I appreciate the explicitness of having Full, and I kind of regret that we stabilized with an implementation on String.

So, part of me resists adding it to any other standard buffer types.

But I also understand that one exists, so should we just swallow the pill and provide the rest? Maybe. I don't love it. But perhaps I'm just wrong.

@akneni
Copy link
Author

akneni commented Aug 27, 2024

I've launched a poll here #3747 . I say we give it a little bit and see what others think.

@akneni
Copy link
Author

akneni commented Sep 5, 2024

It seems like 80% of people (granted, only out of 5 responses) want the body trait implemented on additional types. I can get started working on this if that's good with you @seanmonstar

@dswij
Copy link
Member

dswij commented Sep 6, 2024

I've been personally torn on this. I appreciate the explicitness of having Full, and I kind of regret that we stabilized with an implementation on String.

So, part of me resists adding it to any other standard buffer types.

But I also understand that one exists, so should we just swallow the pill and provide the rest? Maybe. I don't love it. But perhaps I'm just wrong.

In hindsight, it makes sense to at least have an implementation for bytes if we have one for String.

@seanmonstar seanmonstar added the B-rfc Blocked: More comments would be useful in determine next steps. label Sep 24, 2024
@zaira-bibi
Copy link

Are there any plans to have an implementation for more native data types? If yes, then I would love to work on this.

@akneni
Copy link
Author

akneni commented Oct 10, 2024

I've already implemented this trait for Vec<u8>, but no other native types. However, it has yet to be merged, so I think the maintainers are still hesitant to extend this past the string type.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B-rfc Blocked: More comments would be useful in determine next steps. C-feature Category: feature. This is adding a new feature.
Projects
None yet
Development

No branches or pull requests

4 participants