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

Change NewService to MakeService with connection context #1692

Merged
merged 1 commit into from
Nov 16, 2018

Conversation

seanmonstar
Copy link
Member

This adjusts the way Services are created for a hyper::Server. The
MakeService trait allows receiving an argument when creating a
Service. The implementation for hyper::Server expects to pass a
reference to the accepted transport (so, &Incoming::Item). The user
can inspect the transport before making a Service.

In practice, this allows for things like getting the remote socket
address, or the TLS certification, or similar.

To prevent a breaking change, there is a blanket implementation of
MakeService for any NewService. Besides implementing MakeService
directly, there is also added hyper::service::make_service_fn.

Closes #1650

@seanmonstar seanmonstar force-pushed the new-service-ctx branch 2 times, most recently from e191cb4 to 19a573a Compare November 2, 2018 21:20
…ntext

This adjusts the way `Service`s are created for a `hyper::Server`. The
`MakeService` trait allows receiving an argument when creating a
`Service`. The implementation for `hyper::Server` expects to pass a
reference to the accepted transport (so, `&Incoming::Item`). The user
can inspect the transport before making a `Service`.

In practice, this allows for things like getting the remote socket
address, or the TLS certification, or similar.

To prevent a breaking change, there is a blanket implementation of
`MakeService` for any `NewService`. Besides implementing `MakeService`
directly, there is also added `hyper::service::make_service_fn`.

Closes #1650
@sfackler
Copy link
Contributor

sfackler commented Nov 5, 2018

I mildly prefer NewService2 to something like MakeService for this kind of thing - it more clearly identifies the new thing as just the replacement for the old and so reduces potential confusion as to which one to work with.

@seanmonstar
Copy link
Member Author

@sfackler it's to match what's discussed here, that the name NewService may be less ideal than MakeService.

@sfackler
Copy link
Contributor

sfackler commented Nov 5, 2018

Ah cool, that makes sense 👍

@seanmonstar seanmonstar merged commit 3087002 into master Nov 16, 2018
@seanmonstar seanmonstar deleted the new-service-ctx branch November 16, 2018 21:18
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.

2 participants