Skip to content

Use Body types of http-body #557

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.

Sign up for GitHub

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

Jump to bottom
Closed
aeryz opened this issue Feb 7, 2021 · 0 comments · Fixed by #622
Closed

Use Body types of http-body #557

aeryz opened this issue Feb 7, 2021 · 0 comments · Fixed by #622
Labels
A-tonic C-cleanup Category: PRs that clean code up or issues documenting cleanup.

Comments

@aeryz
Copy link

aeryz commented Feb 7, 2021

Feature Request

Crates

tonic

Motivation

Tonic uses its own Body implementations BoxBody, MapBody and EmptyBody. This RFC suggests to add BoxBody, EmptyBody and several others to Hyper. But it seems like they will be added to http-body. There is already a PR opened. After this PR gets merged, and other Body types are implemented, I think tonic should re-export them instead of re-implementing them.

Proposal

Body types from http-body should be re-exported.
@davidpdrsn davidpdrsn added the S-blocked Status: marked as blocked ❌ on something else such as a PR or other implementation work. label Feb 13, 2021
davidpdrsn added a commit that referenced this issue May 1, 2021
@davidpdrsn
tonic: Use BoxBody from http-body crate
3486d80 
As of `http-body` 0.4.1 its has had a `BoxBody` type similar to
`tonic::body::BoxBody`. It also has `Empty` and `Body::map_{data,err}`.

That means all the custom body things we had in tonic can basically be
replaced with that.

Note that this is a breaking change so we should merge this next time we
decide to ship a breaking release.

The breaking changes are:

- `tonic::body::Body` has been removed. I think its fine for users to
depend directly on `http-body` if they need this trait.
- `tonic::body::BoxBody` is now just a type alias for
`http_body::combinators::BoxBody<Bytes, Status>`. So the methods it
previously had are gone. The replacements are
  - `tonic::body::Body::new` -> `http_body::Body::boxed`
  - `tonic::body::Body::map_from` -> `http_body::Body::map_data` and
  `http_body::Body::map_err` depending on which part you want to map.
  - `tonic::body::Body::empty` -> `http_body::Empty`

Additionally a `Sync` bound has been added to a few methods. I actually
don't think this is a breaking change because the old
`tonic::body::Body` trait had `Sync` as a supertrait meaning the `Sync`
requirement was already there.

Fixes #557
@davidpdrsn davidpdrsn mentioned this issue May 1, 2021
Merged
@davidpdrsn davidpdrsn added A-tonic C-cleanup Category: PRs that clean code up or issues documenting cleanup. and removed S-blocked Status: marked as blocked ❌ on something else such as a PR or other implementation work. labels May 1, 2021
davidpdrsn added a commit that referenced this issue May 8, 2021
@davidpdrsn
tonic: Use BoxBody from http-body crate
1f14b9e 
As of `http-body` 0.4.1 its has had a `BoxBody` type similar to
`tonic::body::BoxBody`. It also has `Empty` and `Body::map_{data,err}`.

That means all the custom body things we had in tonic can basically be
replaced with that.

Note that this is a breaking change so we should merge this next time we
decide to ship a breaking release.

The breaking changes are:

- `tonic::body::Body` has been removed. I think its fine for users to
depend directly on `http-body` if they need this trait.
- `tonic::body::BoxBody` is now just a type alias for
`http_body::combinators::BoxBody<Bytes, Status>`. So the methods it
previously had are gone. The replacements are
  - `tonic::body::Body::new` -> `http_body::Body::boxed`
  - `tonic::body::Body::map_from` -> `http_body::Body::map_data` and
  `http_body::Body::map_err` depending on which part you want to map.
  - `tonic::body::Body::empty` -> `http_body::Empty`

Additionally a `Sync` bound has been added to a few methods. I actually
don't think this is a breaking change because the old
`tonic::body::Body` trait had `Sync` as a supertrait meaning the `Sync`
requirement was already there.

Fixes #557
davidpdrsn added a commit that referenced this issue May 18, 2021
@davidpdrsn
feat(tonic): Use BoxBody from http-body crate (#622)
4dda4cb 
As of `http-body` 0.4.1 its has had a `BoxBody` type similar to
`tonic::body::BoxBody`. It also has `Empty` and `Body::map_{data,err}`.

That means all the custom body things we had in tonic can basically be
replaced with that.

Note that this is a breaking change so we should merge this next time we
decide to ship a breaking release.

The breaking changes are:

- `tonic::body::Body` has been removed. I think its fine for users to
depend directly on `http-body` if they need this trait.
- `tonic::body::BoxBody` is now just a type alias for
`http_body::combinators::BoxBody<Bytes, Status>`. So the methods it
previously had are gone. The replacements are
  - `tonic::body::Body::new` -> `http_body::Body::boxed`
  - `tonic::body::Body::map_from` -> `http_body::Body::map_data` and
  `http_body::Body::map_err` depending on which part you want to map.
  - `tonic::body::Body::empty` -> `http_body::Empty`

Additionally a `Sync` bound has been added to a few methods. I actually
don't think this is a breaking change because the old
`tonic::body::Body` trait had `Sync` as a supertrait meaning the `Sync`
requirement was already there.

Fixes #557
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-tonic C-cleanup Category: PRs that clean code up or issues documenting cleanup.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat(tonic): Use BoxBody from http-body crate hyperium/tonic
2 participants
@davidpdrsn @aeryz