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

Consider adding an argument to NewService #108

Closed
carllerche opened this issue Oct 11, 2018 · 6 comments
Closed

Consider adding an argument to NewService #108

carllerche opened this issue Oct 11, 2018 · 6 comments

Comments

@carllerche
Copy link
Member

This has been requested in the past by @olix0r.

Doing so would allow providing additional context. Given that NewService is most often called to process a new connection, the argument could be used to pass connection level data for the service to consume.

Ideas:

@carllerche
Copy link
Member Author

If this change is done, the best way to go about it will probably be to represent NewService as a "service of services". In other words, to implement a NewService, one would implement Service such that Response: Service.

To ease usage of this trait, the main "tower" crate can have a NewService "trait alias".

Also, what do people think about renaming NewService -> MakeService and use the Make prefix for "factories" instead of New.

The main problem I have is, what does this return:

fn build_new_service() -> ....

Does it return T: Service or T: NewService?

fn build_make_service() is a bit clearer.

Thoughts @seanmonstar @olix0r?

@seanmonstar
Copy link
Collaborator

The name NewServivce does cause a little confusion. I'm fine with MakeService, I think.

@seanmonstar
Copy link
Collaborator

To change NewService to basically be a Service of services, it seems making a new service would require mutable access (calling self.call inside self.make_service).

@seanmonstar
Copy link
Collaborator

There's also Service::poll_ready, which suggests there needing to be a MakeService::poll_ready. I actually think that makes sense, and allows for a server to say it cannot handle any more connections for now...

@carllerche
Copy link
Member Author

@seanmonstar yes, I think poll_ready makes sense, the question is about &mut self. However, &mut self makes sense with poll_ready.

@seanmonstar
Copy link
Collaborator

We did this in #114.

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

No branches or pull requests

2 participants