Skip to content

Commit a4fc805

Browse files
Minor refactor of api_client.go to better highlight the client's default value. (#1302)
## What changes are proposed in this pull request? This PR is a no-op that isolates the logic to set default values from the rest of the constructor. The goal is to make the default value more apparent. ## How is this tested? Unit + Integration tests. NO_CHANGELOG=true
1 parent 35d7566 commit a4fc805

1 file changed

Lines changed: 28 additions & 16 deletions

File tree

httpclient/api_client.go

Lines changed: 28 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -104,24 +104,43 @@ func (cfg ClientConfig) httpTransport() http.RoundTripper {
104104
return defaultTransport
105105
}
106106

107-
func NewApiClient(cfg ClientConfig) *ApiClient {
108-
// Set defaults for config values that are not set.
109-
cfg.DebugTruncateBytes = orDefault(cfg.DebugTruncateBytes, 96)
110-
cfg.RetryTimeout = time.Duration(orDefault(int(cfg.RetryTimeout), int(5*time.Minute)))
111-
cfg.HTTPTimeout = time.Duration(orDefault(int(cfg.HTTPTimeout), int(60*time.Second)))
112-
cfg.RateLimitPerSecond = orDefault(cfg.RateLimitPerSecond, 15)
107+
const (
108+
defaultHTTPTimeout = 60 * time.Second
109+
defaultRetryTimeout = 5 * time.Minute
110+
defaultDebugTruncateBytes = 96
111+
defaultRateLimitPerSeconds = 15
112+
)
113113

114+
func setDefaults(cfg *ClientConfig) {
115+
if cfg.HTTPTimeout == 0 {
116+
cfg.HTTPTimeout = defaultHTTPTimeout
117+
}
118+
if cfg.RetryTimeout == 0 {
119+
cfg.RetryTimeout = defaultRetryTimeout
120+
}
121+
if cfg.DebugTruncateBytes == 0 {
122+
cfg.DebugTruncateBytes = defaultDebugTruncateBytes
123+
}
114124
if cfg.ErrorMapper == nil {
115-
// default generic error mapper
116125
cfg.ErrorMapper = DefaultErrorMapper
117126
}
118127
if cfg.ErrorRetriable == nil {
119-
// by default, we just retry on HTTP 429/504
120128
cfg.ErrorRetriable = DefaultErrorRetriable
121129
}
130+
if cfg.RateLimitPerSecond == 0 {
131+
cfg.RateLimitPerSecond = defaultRateLimitPerSeconds
132+
}
133+
}
134+
135+
func NewApiClient(cfg ClientConfig) *ApiClient {
136+
setDefaults(&cfg)
137+
122138
transport := cfg.httpTransport()
123139
rateLimit := rate.Limit(cfg.RateLimitPerSecond)
124-
// depend on the HTTP fixture interface to prevent any coupling
140+
141+
// Depend on the HTTP fixture interface to prevent any coupling.
142+
//
143+
// TODO: Evaluate whether this can be implemented in a cleaner way.
125144
if skippable, ok := transport.(interface {
126145
SkipRetryOnIO() bool
127146
}); ok && skippable.SkipRetryOnIO() {
@@ -411,10 +430,3 @@ func (c *ApiClient) perform(
411430
}
412431
return resp, nil
413432
}
414-
415-
func orDefault(configured, _default int) int {
416-
if configured == 0 {
417-
return _default
418-
}
419-
return configured
420-
}

0 commit comments

Comments
 (0)