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

Provide convenience around serving ServiceFn #2154

Open
rylev opened this issue Mar 18, 2020 · 7 comments
Open

Provide convenience around serving ServiceFn #2154

rylev opened this issue Mar 18, 2020 · 7 comments

Comments

@rylev
Copy link

rylev commented Mar 18, 2020

When making a service to pass to server I've never used make_service_fn in any other way than:

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

It would be nice to be able to write:

let server = Server::bind(&addr).serve(service_fn(handler));

or at least:

let server = Server::bind(&addr).serve(serve_service_fn(service_fn(handler)));

Thoughts?

@rylev rylev changed the title Provide convenience around serving ServiceFn<fn(http::request::Request<hyper::body::body::Body>) -> impl std::future::Future {handler}, hyper::body::body::Body> Provide convenience around serving ServiceFn Mar 18, 2020
@seanmonstar
Copy link
Member

I get what you mean, I even had filed something similar a while ago and never gotten back to it: tower-rs/tower#262

@davidpdrsn
Copy link
Member

Tower recently got tower::make::Shared which changes this

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

Into this

let server = Server::bind(&addr).serve(Shared::new(service_fn(handler));

There is also an example in the hyper docs here https://docs.rs/hyper/0.14.7/hyper/server/index.html#example

@rylev Do you think this is sufficient to close this?

@rylev
Copy link
Author

rylev commented May 5, 2021

I would have preferred if Hyper had a solution that didn't require taking another dependency. The example is still not what I would call easy and intuitive to understand.

@davidpdrsn
Copy link
Member

Thats true. Having to pull in tower is unfortunate. I'm working on a proper fix.

davidpdrsn added a commit that referenced this issue May 6, 2021
This adds `Server::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 added a commit that referenced this issue May 6, 2021
This adds `Server::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 added a commit that referenced this issue May 9, 2021
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
@dcecile
Copy link

dcecile commented Nov 5, 2021

The new serve_service looks good 👍 (For now I'm just using Shared::new directly.)

What do you think about updating the "passing data" example or adding an extra example?

Original:

//!     let make_service = make_service_fn(move |conn: &AddrStream| {
//!         // We have to clone the context to share it with each invocation of
//!         // `make_service`. If your data doesn't implement `Clone` consider using
//!         // an `std::sync::Arc`.
//!         let context = context.clone();
//!
//!         // You can grab the address of the incoming connection like so.
//!         let addr = conn.remote_addr();
//!
//!         // Create a `Service` for responding to the request.
//!         let service = service_fn(move |req| {
//!             handle(context.clone(), addr, req)
//!         });
//!
//!         // Return the service to hyper.
//!         async move { Ok::<_, Infallible>(service) }
//!     });
//!
//!     // Run the server like above...
//!     let addr = SocketAddr::from(([127, 0, 0, 1], 3000));
//!
//!     let server = Server::bind(&addr).serve(make_service);

I think it can be simplified to this (assuming conn/addr isn't needed):

//!     let service = service_fn(move |req| {
//!         handle(context.clone(), req)
//!     });
//!
//!     // Run the server like above...
//!     let addr = SocketAddr::from(([127, 0, 0, 1], 3000));
//!
//!     let server = Server::bind(&addr).serve_service(service);

@davidpdrsn
Copy link
Member

We ended up not merging that PR #2541

@dcecile
Copy link

dcecile commented Nov 5, 2021

😅 Oh I see! (Checking out #2321 linked in the PR...)

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 a pull request may close this issue.

4 participants