Skip to content

Commit

Permalink
code review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
tejal29 committed Jun 24, 2020
1 parent 6649a5f commit 5357cf4
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 8 deletions.
5 changes: 4 additions & 1 deletion cmd/skaffold/app/cmd/commands.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,8 @@ func (b *builder) ExactArgs(argCount int, action func(context.Context, io.Writer
b.cmd.Args = cobra.ExactArgs(argCount)
b.cmd.RunE = func(_ *cobra.Command, args []string) error {
err := handleWellKnownErrors(action(b.cmd.Context(), b.cmd.OutOrStdout(), args))
// clean up server.
// clean up server at end of the execution since post run hooks are only executed if
// RunE is successful
if shutdownAPIServer != nil {
shutdownAPIServer()
}
Expand All @@ -102,6 +103,8 @@ func (b *builder) NoArgs(action func(context.Context, io.Writer) error) *cobra.C
b.cmd.Args = cobra.NoArgs
b.cmd.RunE = func(*cobra.Command, []string) error {
err := handleWellKnownErrors(action(b.cmd.Context(), b.cmd.OutOrStdout()))
// clean up server at end of the execution since post run hooks are only executed if
// RunE is successful
if shutdownAPIServer != nil {
shutdownAPIServer()
}
Expand Down
13 changes: 6 additions & 7 deletions pkg/skaffold/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ import (
var (
srv *server

// waits for 1 second before shutting down the server
secondTimeout = 1 * time.Second
// waits for 1 second before forcing a server shutdown
forceShutdownTimeout = 1 * time.Second
)

type server struct {
Expand Down Expand Up @@ -161,7 +161,7 @@ func newGRPCServer(port int) (func() error, error) {
}
}()
return func() error {
ctx, cancel := context.WithTimeout(context.Background(), secondTimeout)
ctx, cancel := context.WithTimeout(context.Background(), forceShutdownTimeout)
defer cancel()
ch := make(chan bool, 1)
go func() {
Expand Down Expand Up @@ -194,14 +194,13 @@ func newHTTPServer(port, proxyPort int) (func() error, error) {
logrus.Infof("starting gRPC HTTP server on port %d", port)

server := &http.Server{
Handler: mux,
ReadTimeout: 10 * time.Second,
Handler: mux,
}

go http.Serve(l, mux)
go server.Serve(l)

return func() error {
ctx, cancel := context.WithTimeout(context.Background(), secondTimeout)
ctx, cancel := context.WithTimeout(context.Background(), forceShutdownTimeout)
defer cancel()
return server.Shutdown(ctx)
}, nil
Expand Down

0 comments on commit 5357cf4

Please sign in to comment.