-
Notifications
You must be signed in to change notification settings - Fork 1k
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(transport): Add server graceful shutdown #169
Conversation
Some error messages, one about hyper::server::Builder not existing and another about lack of clone on futures. Just putting this up as an initial WIP to make sure I'm on the right track :)
tonic/src/transport/server.rs
Outdated
@@ -110,6 +111,14 @@ impl Server { | |||
} | |||
} | |||
|
|||
/// Set the graceful shutdown thing | |||
pub fn graceful_shutdown(self, shutdown: Box<dyn Future<Output=()>>) -> Self { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd probably make this impl Future<Output = ()>
and then box it internally.
Also fill in docs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks!
hmm looks like there are some compile errors? |
Co-Authored-By: Lucio Franco <luciofranco14@gmail.com>
Yeah |
Oh wait I did change something! Sorting it now |
Issues wth sizes I can't know the size of so moved to try something else
Going back to the old way but now with pins and boxes. Issues with Option requiring Clone and Unpin for dyn Future
@LucioFranco did you have a chance to look at my type based woes? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thank you!
Looks like there are a couple of CI failures, mind fixing them, then we can merge this. |
Yeah but I'm not sure quite how, my attempt to make serve_common to avoid repeating code leads to |
Can't get past generic type restricts on async functions so just went for the code duplication approach in the interest of getting something working
Some error messages, one about hyper::server::Builder not existing and another about lack of clone on futures. Just putting this up as an initial WIP to make sure I'm on the right track :)
Motivation
Recommended good practice for gRPC so should be exposed
Solution
Expose it via builder pattern