From ca424d5d5ed86fc308b2df3627fbc5ea01b6c04b Mon Sep 17 00:00:00 2001 From: Katrina Owen Date: Sun, 26 Aug 2018 11:59:12 -0600 Subject: [PATCH 1/5] Rename default http client in api package for consistency The cli package uses just HTTPClient for this. --- api/client.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/api/client.go b/api/client.go index 22e483419..af2cffa2b 100644 --- a/api/client.go +++ b/api/client.go @@ -14,8 +14,8 @@ var ( // It's overridden from the root command so that we can set the version. UserAgent = "github.com/exercism/cli" - // DefaultHTTPClient configures a timeout to use by default. - DefaultHTTPClient = &http.Client{Timeout: 10 * time.Second} + // HTTPClient configures a timeout to use by default. + HTTPClient = &http.Client{Timeout: 10 * time.Second} ) // Client is an http client that is configured for Exercism. @@ -29,7 +29,7 @@ type Client struct { // NewClient returns an Exercism API client. func NewClient(token, baseURL string) (*Client, error) { return &Client{ - Client: DefaultHTTPClient, + Client: HTTPClient, Token: token, APIBaseURL: baseURL, }, nil @@ -38,7 +38,7 @@ func NewClient(token, baseURL string) (*Client, error) { // NewRequest returns an http.Request with information for the Exercism API. func (c *Client) NewRequest(method, url string, body io.Reader) (*http.Request, error) { if c.Client == nil { - c.Client = DefaultHTTPClient + c.Client = HTTPClient } req, err := http.NewRequest(method, url, body) From 12c3d2fb2c30ff0ec129f6a874650867bd8dbf9d Mon Sep 17 00:00:00 2001 From: Katrina Owen Date: Sun, 26 Aug 2018 12:01:47 -0600 Subject: [PATCH 2/5] Make HTTP timeout configurable --- api/client.go | 6 ++++-- cli/cli.go | 4 +++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/api/client.go b/api/client.go index af2cffa2b..5fc67d1e3 100644 --- a/api/client.go +++ b/api/client.go @@ -14,8 +14,10 @@ var ( // It's overridden from the root command so that we can set the version. UserAgent = "github.com/exercism/cli" - // HTTPClient configures a timeout to use by default. - HTTPClient = &http.Client{Timeout: 10 * time.Second} + // TimeoutInSeconds is the timeout the default HTTP client will use. + TimeoutInSeconds = 10 + // HTTPClient is the client used to make HTTP calls in the cli package. + HTTPClient = &http.Client{Timeout: time.Duration(TimeoutInSeconds) * time.Second} ) // Client is an http client that is configured for Exercism. diff --git a/cli/cli.go b/cli/cli.go index b20e1ebbe..da6601b1c 100644 --- a/cli/cli.go +++ b/cli/cli.go @@ -43,8 +43,10 @@ var ( ) var ( + // TimeoutInSeconds is the timeout the default HTTP client will use. + TimeoutInSeconds = 10 // HTTPClient is the client used to make HTTP calls in the cli package. - HTTPClient = &http.Client{Timeout: 10 * time.Second} + HTTPClient = &http.Client{Timeout: time.Duration(TimeoutInSeconds) * time.Second} // ReleaseURL is the endpoint that provides information about cli releases. ReleaseURL = "https://api.github.com/repos/exercism/cli/releases" ) From 5673f90ab2cc3181e51e98a59721554876b66b95 Mon Sep 17 00:00:00 2001 From: Katrina Owen Date: Sun, 26 Aug 2018 12:02:24 -0600 Subject: [PATCH 3/5] Configure custom HTTP timeout in troubleshoot command This configures the timeout on the cli package rather than making a whole new client. --- cmd/troubleshoot.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/cmd/troubleshoot.go b/cmd/troubleshoot.go index a205a6783..363e22559 100644 --- a/cmd/troubleshoot.go +++ b/cmd/troubleshoot.go @@ -4,7 +4,6 @@ import ( "bytes" "fmt" "html/template" - "net/http" "runtime" "strings" "sync" @@ -30,7 +29,7 @@ If you're running into trouble, copy and paste the output from the troubleshoot command into a GitHub issue so we can help figure out what's going on. `, RunE: func(cmd *cobra.Command, args []string) error { - cli.HTTPClient = &http.Client{Timeout: 20 * time.Second} + cli.TimeoutInSeconds = 20 c := cli.New(Version) cfg := config.NewConfig() From 9c591874d5ab8fe7d2c24223c627cec6361e29f6 Mon Sep 17 00:00:00 2001 From: Katrina Owen Date: Sun, 26 Aug 2018 12:14:25 -0600 Subject: [PATCH 4/5] Add persistent flag for timeout override In the root command, configure a persistent timeout flag, and check it in the 'prerun' logic, overriding the default value if it isn't the zerovalue for the flag. --- cmd/root.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/cmd/root.go b/cmd/root.go index 1b0d9a139..e2b09a423 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -7,6 +7,7 @@ import ( "runtime" "github.com/exercism/cli/api" + "github.com/exercism/cli/cli" "github.com/exercism/cli/config" "github.com/exercism/cli/debug" "github.com/spf13/cobra" @@ -39,6 +40,10 @@ Download exercises and submit your solutions.`, if verbose, _ := cmd.Flags().GetBool("verbose"); verbose { debug.Verbose = verbose } + if timeout, _ := cmd.Flags().GetInt("timeout"); timeout > 0 { + cli.TimeoutInSeconds = timeout + api.TimeoutInSeconds = timeout + } }, } @@ -57,4 +62,5 @@ func init() { In = os.Stdin api.UserAgent = fmt.Sprintf("github.com/exercism/cli v%s (%s/%s)", Version, runtime.GOOS, runtime.GOARCH) RootCmd.PersistentFlags().BoolP("verbose", "v", false, "verbose output") + RootCmd.PersistentFlags().IntP("timeout", "", 0, "override the default HTTP timeout (seconds)") } From bf809ab45d7e7ce223d095afc6d02702760fe928 Mon Sep 17 00:00:00 2001 From: Katrina Owen Date: Mon, 27 Aug 2018 19:55:22 -0600 Subject: [PATCH 5/5] Tweak timeouts based on feedback from code review --- api/client.go | 2 +- cli/cli.go | 2 +- cmd/troubleshoot.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/api/client.go b/api/client.go index 5fc67d1e3..103df119e 100644 --- a/api/client.go +++ b/api/client.go @@ -15,7 +15,7 @@ var ( UserAgent = "github.com/exercism/cli" // TimeoutInSeconds is the timeout the default HTTP client will use. - TimeoutInSeconds = 10 + TimeoutInSeconds = 60 // HTTPClient is the client used to make HTTP calls in the cli package. HTTPClient = &http.Client{Timeout: time.Duration(TimeoutInSeconds) * time.Second} ) diff --git a/cli/cli.go b/cli/cli.go index da6601b1c..29f6d64a3 100644 --- a/cli/cli.go +++ b/cli/cli.go @@ -44,7 +44,7 @@ var ( var ( // TimeoutInSeconds is the timeout the default HTTP client will use. - TimeoutInSeconds = 10 + TimeoutInSeconds = 60 // HTTPClient is the client used to make HTTP calls in the cli package. HTTPClient = &http.Client{Timeout: time.Duration(TimeoutInSeconds) * time.Second} // ReleaseURL is the endpoint that provides information about cli releases. diff --git a/cmd/troubleshoot.go b/cmd/troubleshoot.go index 363e22559..12f2277e3 100644 --- a/cmd/troubleshoot.go +++ b/cmd/troubleshoot.go @@ -29,7 +29,7 @@ If you're running into trouble, copy and paste the output from the troubleshoot command into a GitHub issue so we can help figure out what's going on. `, RunE: func(cmd *cobra.Command, args []string) error { - cli.TimeoutInSeconds = 20 + cli.TimeoutInSeconds = cli.TimeoutInSeconds * 2 c := cli.New(Version) cfg := config.NewConfig()