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): Creating Server with a Handle WIP #1332

Closed
wants to merge 2 commits into from
Closed

feat(server): Creating Server with a Handle WIP #1332

wants to merge 2 commits into from

Conversation

Thomspoon
Copy link

@Thomspoon Thomspoon commented Sep 22, 2017

This PR is a work-in-progress for #1322.

Copy link
Member

@seanmonstar seanmonstar left a comment

Choose a reason for hiding this comment

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

Great start!

I believe the code inside run_until can be moved to impl Future for Server, and then run_until can convert the Server from one that ones a Core to one that owns a Handle from that same core, and just calls core.run(server). (Of course, still a panic if the Server didn't have a Core).

@@ -389,7 +416,10 @@ impl<S, B> Server<S, B>
/// Returns a handle to the underlying event loop that this server will be
/// running on.
pub fn handle(&self) -> Handle {
self.core.handle()
match self.internal {
ServerInternal::Core(ref core) => core.handle().clone(),
Copy link
Member

Choose a reason for hiding this comment

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

The clone here isn't need, core.handle() already returns Handle.

where S: NewService<Request = Request, Response = Response<Bd>, Error = ::Error> + 'static,
Bd: Stream<Item=B, Error=::Error>,
{
let listener = TcpListener::bind(addr, &handle).unwrap();
Copy link
Member

Choose a reason for hiding this comment

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

This probably means bind_handle should return a Result, just like bind does, so that we don't panic.

@seanmonstar
Copy link
Member

Master has grown to have a Serve type which will be used to explore replacing Server. Serve doesn't require a Core, but instead is a stream of binding connections with HTTP.

Thanks for taking a stab at this!

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