Refactor Server customize shutdown and signals to support embedding #493
Conversation
kriswuollett
force-pushed
the
signal-handling-fn
branch
2 times, most recently
from
0ae925f to
6773af5
Compare
…ior. This change supports building a server that can run Pingora without it calling std::process::exit during shutdown.
kriswuollett
force-pushed
the
signal-handling-fn
branch
from
6773af5 to
11881c5
Compare
| GracefulTerminate, | ||
| FastShutdown, | ||
| } | ||
|
|
There was a problem hiding this comment.
Please add doc comments for these public types.
pingora-core/src/server/mod.rs
Outdated
| @@ -306,14 +362,25 @@ impl Server { | |||
| } | |||
| } | |||
|
|
|||
| /// Start the server using [Self::run] and default [RunArgs]. | |||
| #[allow(unused_mut)] // TODO: May not need to keep mut self in interface | |||
| FastShutdown, | ||
| } | ||
|
|
||
| #[async_trait] |
There was a problem hiding this comment.
Sorry for the delay, do you mind adding doc comments for these public types as well?
There was a problem hiding this comment.
No problem, added some docs. Let me know if you would like them worded differently.
| /// Shutdown with no timeout for runtime shutdown. | ||
| FastShutdown, | ||
| } | ||
|
|
There was a problem hiding this comment.
It would make sense to derive Debug here.
There was a problem hiding this comment.
Added Debug derive. Not sure if you also want me to go ahead and add to other related ones like ShutdownType?
There was a problem hiding this comment.
Note: I'm not affiliated with Cloudflare, I just started using your changes from here and ran into an issue due to the missing Debug impl.
There was a problem hiding this comment.
What is the issue you encountered?
gumpt
added
Accepted
This change currently aims to minimize the code diff. Still need to test for my use case.
Fixes #492