Add Shutdown method to registry.Registry #4338
Conversation
976913f to
a34e501
Compare
|
Missed some really obvious things. Fixes are pushed, and my own fork is working nicely in my embedding app. |
There was a problem hiding this comment.
I see no reason why we shouldn't accept this change. LGTM. PTAL @thaJeztah
|
Looks like there's some fix-up commits in the PR that should be squashed; can you clean up commit history a bit? |
registry/registry.go
Outdated
| func (registry *Registry) Shutdown() error { | ||
| dcontext.GetLogger(registry.app).Info("stopping server gracefully. Draining connections for ", registry.config.HTTP.DrainTimeout) | ||
| // shutdown the server with a grace period of configured timeout | ||
| c, cancel := context.WithTimeout(context.Background(), registry.config.HTTP.DrainTimeout) |
There was a problem hiding this comment.
As this is new functionality; would it make sense to pass a context to the function; also to make it closer to the server.Shutdown() ?
There was a problem hiding this comment.
I modified to function to accept a context instead of setting its own. Please take a look.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
d4a3696 to
dda410a
Compare
Signed-off-by: Robin Ketelbuters <robin.ketelbuters@gmail.com>
dda410a to
16a305e
Compare
Sorry, I'm used to squashing my PRs. I just did so. |
Signed-off-by: David Karlsson <35727626+dvdksn@users.noreply.github.com>
Adds a Shutdown method to
registry.Registry. Enables gracefully shutting down an embedded registry HTTP server.Closes #4337.