-
Notifications
You must be signed in to change notification settings - Fork 86
README: add gracefully shutdown http server example #36
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
base: main
Are you sure you want to change the base?
Conversation
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.
SignalHandler is orthogonal to the example code that's included in the README, and in any case the package docs should (hopefully!) make clear how to use that particular helper.
I guess it could make sense to add an http.Server.Shutdown example to the README, but it would probably need to look something like my suggestion, here.
### Gracefully shutdown HTTP server | ||
```go | ||
g := &run.Group{} | ||
g.Add(run.SignalHandler(context.Background(), syscall.SIGINT, syscall.SIGTERM)) | ||
srv := &http.Server{} | ||
g.Add(func() error { | ||
log.Printf("starting server") | ||
return srv.ListenAndServe() | ||
}, func(err error) { | ||
log.Printf("shutting down server") | ||
shutdownErr := srv.Shutdown(context.Background()) | ||
log.Printf("shutdown err: %v", shutdownErr) | ||
}) | ||
err := g.Run() | ||
log.Printf("run err: %v", err) | ||
``` | ||
|
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.
### Gracefully shutdown HTTP server | |
```go | |
g := &run.Group{} | |
g.Add(run.SignalHandler(context.Background(), syscall.SIGINT, syscall.SIGTERM)) | |
srv := &http.Server{} | |
g.Add(func() error { | |
log.Printf("starting server") | |
return srv.ListenAndServe() | |
}, func(err error) { | |
log.Printf("shutting down server") | |
shutdownErr := srv.Shutdown(context.Background()) | |
log.Printf("shutdown err: %v", shutdownErr) | |
}) | |
err := g.Run() | |
log.Printf("run err: %v", err) | |
``` | |
### http.Server.Shutdown | |
```go | |
srv := &http.Server{ | |
Addr: "localhost:8080", | |
Handler: ..., | |
} | |
g.Add(func() error { | |
return srv.ListenAndServe() | |
}, func(error) { | |
ctx, cancel := context.WithTimeout(context.TODO(), 3*time.Second) | |
defer cancel() | |
srv.Shutdown(ctx) | |
}) |
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.
SignalHandler is orthogonal to the example code that's included in the README, and in any case the package docs should (hopefully!) make clear how to use that particular helper.
Yes, you’re right—this SignalHandler
is orthogonal to the code in Readme. In my opinion, the examples currently only shows the control flow of run
but doesn’t demonstrate the helper (SignalHandler
).
I’d like to add it to the README because:
- Some users only skim the README to see if it has the features they need before diving into the package documentation for details. That’s why most authors include real-world examples in the README. For instance, in this case, we could show users that
run
provides aSignalHandler
. I believe most users aren’t aware of this feature—at least my colleagues aren’t. - Gracefully shutting down an HTTP server is a commonly used piece of code, and I’d like users to know that
run
can accomplish this too.
That said, I’m totally okay with your approach as well, and I think it could work just fine. What do you think about proceeding with this?
run
provides a good way to write a gracefully shutting down HTTP server. It also provides aSignalHandler
to make it more elegant. So I want to add this example in the Readme.What's changed