hyper
hyper copied to clipboard
feat: create Service trait
Closes https://github.com/hyperium/hyper/issues/2853.
Ok, I see. You could leave the tower-service
dependency in for now, since the work to transfer this client stuff is in a separate PR, and then this PR is just replacing the server side. Then when both PRs are done, we can chop out of the Cargo.toml
. I think that'll be the least complicated as all this movement happens.
I might be doing something wrong, but it's impossible to just replace the server
part. It would require maintaining duplicates of most parts of src/service
and some parts src/common
, one with stuff that implements tower_service::Service
and one with hyper::service::Service
. The clean sweep throughout the repo was the closest I got to compiling, working code. Trying to do anything local to src/server
just keeps leaking to other parts of the repo.
@seanmonstar I could use some guidance here. I saw you did some work on removing tcp
feature and parts of src/client
. Should I maybe wait till client
part is completely gone? I think clean sweep remove of tower_service
dependency is the best way to move this MR forward.
Alright, with the help of cross-cutting removals of code Sean have done to the client
module, this PR was way easier to do.
Looks good to me now, @seanmonstar I'd ask for CI run and if it's all green (except for miri
) a review.
Dumb mistake, I fought the compiler for so long I forgot to run the tests once the project finally compiled.
Locally, all the tests (including examples) pass now.
There is only one reference to tower
left in the whole project, and it's in client/conn/mod.rs
here. We could easily make SendRequest
implement hyper::service::Service
instead and remove the dependency all together. It would be a clean sweep of tower/tower-service
from the project.
I can do fmt + check
or test
, but I seem to be uncapable of remembering to run all 3 of them before pushing :smile:
Alright, couldn't stop myself from removing the last tower
reference from client::conn::SendRequest
. The repo is now all free of the dependency. If you think that's wrong, I can revert the last commit.
I'm certain pipelines will pass, so I'll wait for the decision and CR.
Juuust one more CI trigger.
@seanmonstar Could you address https://github.com/hyperium/hyper/pull/2920#discussion_r950670503 and https://github.com/hyperium/hyper/pull/2920#discussion_r950680044, and I'll move this PR forward?
@seanmonstar I introduced changes from the discussion threads.
LGTM, let's wrap and merge this as soon as possible, as the merge conflicts on this branch are killing me :stuck_out_tongue_winking_eye:
CI failed on a doc-test, this should be fixed now.
Thanks so so so much! I know this dragged on a while, but it turns out there were tricky things for us to figure out (and conflicts as all sorts of other things got yanked).