Skip to content

fix(server): make tower::Service impl generic over HttpBody#1475

Merged
niklasad1 merged 10 commits intoparitytech:masterfrom
hanabi1224:compression-layer
Oct 16, 2024
Merged

fix(server): make tower::Service impl generic over HttpBody#1475
niklasad1 merged 10 commits intoparitytech:masterfrom
hanabi1224:compression-layer

Conversation

@hanabi1224
Copy link
Contributor

@hanabi1224 hanabi1224 commented Oct 16, 2024

Close #1133

  • fix build errors when using tower_http::compression::CompressionLayer as http middleware
  • update examples
  • upgrade tower and tower-http

@hanabi1224 hanabi1224 changed the title fix: compatibility issue of CompressionLayer in http middleware fix(server): compatibility issue of CompressionLayer in http middleware Oct 16, 2024
@hanabi1224 hanabi1224 marked this pull request as ready for review October 16, 2024 13:20
@hanabi1224 hanabi1224 requested a review from a team as a code owner October 16, 2024 13:20
}

impl<Body, RpcMiddleware, HttpMiddleware> Service<HttpRequest<Body>> for TowerService<RpcMiddleware, HttpMiddleware>
impl<RequestBody, ResponseBody, RpcMiddleware, HttpMiddleware> Service<HttpRequest<RequestBody>> for TowerService<RpcMiddleware, HttpMiddleware>
Copy link
Contributor

@niklasad1 niklasad1 Oct 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, this is the fix right i.e, to make it generic over the body?

I would rather just include this fix and avoid updating tower so we don't have make breaking release for this but looks good to me overall and fix it for folks of v0.24.x release as well

Then release v0.25 as well with tower 0.5

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@niklasad1 thanks for your quick review! Just reverted the tower* upgrade, will do that in a separate PR. BTW, should I bump the jsonrpsee patch versions as well?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usually we do a separate release PR for such things but since you already done it's fine. Nice work

@niklasad1 niklasad1 changed the title fix(server): compatibility issue of CompressionLayer in http middleware fix(server): make tower::Service impl generic over HttpBody Oct 16, 2024
@niklasad1
Copy link
Contributor

@hanabi1224 one tiny thing, can you just change the links in the README from 0.24.6 to 0.24.7?

Copy link
Contributor

@niklasad1 niklasad1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice one, thank you

@hanabi1224
Copy link
Contributor Author

@hanabi1224 one tiny thing, can you just change the links in the README from 0.24.6 to 0.24.7?

@niklasad1 Do you mean the dependency status badge? I can change it in this PR but the badge will be down until 0.24.7 is actually published.
[![dependency status](https://deps.rs/crate/jsonrpsee/0.24.7/status.svg)](https://deps.rs/crate/jsonrpsee/0.24.7)

@niklasad1
Copy link
Contributor

Yes, exactly I'll release after this is merged.

@hanabi1224
Copy link
Contributor Author

Yes, exactly I'll release after this is merged.

@niklasad1 Thanks! (Just curious, why not use https://deps.rs/crate/jsonrpsee/latest/status.svg , like [![dependency status](https://deps.rs/crate/jsonrpsee/latest/status.svg)](https://deps.rs/crate/jsonrpsee) ?)

@niklasad1
Copy link
Contributor

niklasad1 commented Oct 16, 2024

@niklasad1 Thanks! (Just curious, why not use https://deps.rs/crate/jsonrpsee/latest/status.svg , like dependency status ?)

Sure, that's better. Changed now

@niklasad1 niklasad1 merged commit 93722cb into paritytech:master Oct 16, 2024
@hanabi1224 hanabi1224 deleted the compression-layer branch October 16, 2024 20:52
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

Successfully merging this pull request may close these issues.

Feature request: http_only compression middleware

2 participants