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

feat(server): add server::Builder::serve_service #2541

Closed
wants to merge 1 commit into from

Conversation

davidpdrsn
Copy link
Member

@davidpdrsn davidpdrsn commented May 6, 2021

This adds server::Builder::serve_service which is a convenience for doing

Server::bind(&addr).serve(make_service_fn(|_| async move {
    Ok::<_, Infallible>(service_fn(handler))
}));

That now becomes

Server::bind(&addr).serve_service(service_fn(handler));

It basically copies tower::make::Shared into Hyper so users don't need
to add another dependency to do this.

Also adjusted the docs for hyper::server a bit to make server_service
the top example as I believe its the more common use-case.

Fixes #2154

@davidpdrsn davidpdrsn added the A-server Area: server. label May 6, 2021
/// }
/// # }
/// ```
pub fn serve_service<S, B>(self, service: S) -> Server<I, Shared<S>, E>
Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think the name serve_service is particularly good but couldn't come up with anything else.

Copy link
Contributor

Choose a reason for hiding this comment

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

serve?

Copy link
Member

Choose a reason for hiding this comment

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

What if instead of a separate method, it's part of hyper::service?

Server::bind(addr)
    .serve(hyper::service::shared(svc))

Copy link
Member Author

Choose a reason for hiding this comment

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

serve?

serve is already taken https://docs.rs/hyper/0.14.7/hyper/server/struct.Builder.html#method.serve 😞 This method is actually not on Server but on server::Builder. Fixing description now.

What if instead of a separate method, it's part of hyper::service?

I was thinking that since its such a common use case it made sense to have a dedicated method for it.

@davidpdrsn davidpdrsn force-pushed the david/add-serve-service branch from c6d6bfd to 4931c8b Compare May 6, 2021 20:29
This adds `server::Builder::serve_service` which is a convenience for doing

```rust
Server::bind(&addr).serve(make_service_fn(|_| async move {
    Ok::<_, Infallible>(service_fn(handler))
}));
```

That now becomes

```rust
Server::bind(&addr).serve_service(service_fn(handler));
```

It basically copies [`tower::make::Shared`][] into Hyper so users don't need
to add another dependency to do this.

Fixes #2154

[`tower::make::Shared`]: https://docs.rs/tower/0.4.7/tower/make/struct.Shared.html
@davidpdrsn davidpdrsn force-pushed the david/add-serve-service branch from 4931c8b to 1f1c03b Compare May 9, 2021 19:17
@davidpdrsn davidpdrsn changed the title feat(server): add Server::serve_service feat(server): add server::Builder::serve_service May 9, 2021
@seanmonstar seanmonstar requested a review from LucioFranco May 19, 2021 17:01
@LucioFranco
Copy link
Member

I think instead of adding more methods to the Server type we should just go ahead and do #2321

@davidpdrsn
Copy link
Member Author

We discussed this in Discord and decided to wait as a bigger change to Server is planned as part of either 0.15 or a potential 1.0.

@davidpdrsn davidpdrsn closed this May 25, 2021
@seanmonstar seanmonstar deleted the david/add-serve-service branch December 1, 2023 13:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-server Area: server.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide convenience around serving ServiceFn
4 participants