Skip to content

Commit

Permalink
fix(client-timeout): change default timeout to no timeout and make su…
Browse files Browse the repository at this point in the history
…re go-api respects it

This has been discussed (Slack link below) and seems like there really is no reason to have any sort of a timeout in the client. Our backend and load balancers already enforce timeouts on the backend side and there seems to be no benefit in timing out a client request in the client end, as that will only throw errors to the users that don't really convey any real information. In particular, storage imports can easily go over any sensible timeouts so the other option would be to have per-command timeouts.

Ref: https://upcloud.slack.com/archives/C01MSUTR6PL/p1617269550033700
  • Loading branch information
moitias committed Apr 22, 2021
1 parent b11df0b commit 0171b12
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 6 deletions.
10 changes: 7 additions & 3 deletions internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,14 +191,18 @@ func (s *Config) CreateService() (internal.AllServices, error) {
return nil, fmt.Errorf("user credentials not found, these must be set in config file (%s) or via environment variables", configDetails)
}

hc := &http.Client{Transport: &transport{}}
hc.Timeout = s.ClientTimeout()

hc := &http.Client{Transport: &transport{}, Timeout: s.ClientTimeout()}
whc := client.NewWithHTTPClient(
username,
password,
hc,
)
// TODO: remove this after go-api actually respects 0 timeout
// this is in order to enforce our custom (no) timeout because currently go-api
// assumes 0 timeout means 'not set' rather than 'no timeout'.
// see https://github.com/UpCloudLtd/upcloud-go-api/blob/2964ed7e597209b50a21f34259a20249e9aa220c/upcloud/client/client.go#L48
hc.Timeout = s.ClientTimeout()

whc.UserAgent = fmt.Sprintf("upctl/%s", Version)
svc := service.New(whc)
// TODO; remove this when refactor is complete
Expand Down
4 changes: 1 addition & 3 deletions internal/core/core.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ package core

import (
"fmt"
"time"

"github.com/UpCloudLtd/upcloud-cli/internal/commands"
"github.com/UpCloudLtd/upcloud-cli/internal/commands/all"
"github.com/UpCloudLtd/upcloud-cli/internal/config"
Expand Down Expand Up @@ -60,7 +58,7 @@ func BuildRootCmd(conf *config.Config) cobra.Command {
)
flags.DurationVarP(
&conf.GlobalFlags.ClientTimeout, "client-timeout", "t",
time.Duration(60*time.Second),
0,
"CLI timeout when using interactive mode on some commands",
)

Expand Down

0 comments on commit 0171b12

Please sign in to comment.