diff options
author | Steve Azzopardi <sazzopardi@gitlab.com> | 2023-01-06 19:35:21 +0100 |
---|---|---|
committer | Ash McKenzie <amckenzie@gitlab.com> | 2023-01-12 02:56:43 +0000 |
commit | 16a5c84310f6bb1790f16b6b2e4df90af493b07c (patch) | |
tree | 5429ecdcc2996b3cf13beddea23671358fca54d3 /client/httpclient_test.go | |
parent | a093c9d3cfc1ee18368ebbf828dc61c15b74540c (diff) | |
download | gitlab-shell-16a5c84310f6bb1790f16b6b2e4df90af493b07c.tar.gz |
feat: put retryablehttp.Client behind feature flag
What
---
- Update the `client.HttpClient` fields to have `http.Client` and
`retryablehttp.Client`, one of them will be `nil` depending on the
feature flag toggle.
- Create new method `newRetryableRequest` which will create a
`retryablehttp.Request` and use that if the
`FF_GITLAB_SHELL_RETRYABLE_HTTP` feature flag is turned on.
- Add checks for `FF_GITLAB_SHELL_RETRYABLE_HTTP` everywhere we use the
http client to use the `retryablehttp.Client` or the default
`http.Client`
- New job `tests-integration-retryableHttp` to run the integraiton tests
with the new retryablehttp client. We didn't update go tests because
some assertions are different and will break table driven tests.
Why
---
As discussed in
https://gitlab.com/gitlab-org/gitlab-shell/-/merge_requests/703#note_1229645097
we want to put the client behind a feature flag, not just the retry
logic. This does bring extra risk for accessing a `nil` field but there
should be checks everytime we access `RetryableHTTP` and `HTTPClient`.
Reference: https://gitlab.com/gitlab-com/gl-infra/production/-/issues/7979
Signed-off-by: Steve Azzopardi <sazzopardi@gitlab.com>
Diffstat (limited to 'client/httpclient_test.go')
-rw-r--r-- | client/httpclient_test.go | 9 |
1 files changed, 7 insertions, 2 deletions
diff --git a/client/httpclient_test.go b/client/httpclient_test.go index a5de47c..ee2c6fd 100644 --- a/client/httpclient_test.go +++ b/client/httpclient_test.go @@ -21,7 +21,13 @@ func TestReadTimeout(t *testing.T) { require.NoError(t, err) require.NotNil(t, client) - require.Equal(t, time.Duration(expectedSeconds)*time.Second, client.HTTPClient.Timeout) + if client.HTTPClient != nil { + require.Equal(t, time.Duration(expectedSeconds)*time.Second, client.HTTPClient.Timeout) + } + + if client.RetryableHTTP != nil { + require.Equal(t, time.Duration(expectedSeconds)*time.Second, client.RetryableHTTP.HTTPClient.Timeout) + } } const ( @@ -117,7 +123,6 @@ func TestRequestWithUserAgent(t *testing.T) { client.SetUserAgent(gitalyUserAgent) _, err = client.Get(context.Background(), "/override_user_agent") require.NoError(t, err) - } func setup(t *testing.T, username, password string, requests []testserver.TestRequestHandler) *GitlabNetClient { |