-
Notifications
You must be signed in to change notification settings - Fork 24
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: retain http.Client config when retries are enabled #157
Conversation
@@ -273,41 +300,9 @@ func (service *BaseService) IsSSLDisabled() bool { | |||
return false | |||
} | |||
|
|||
// getClientTransportForSSL() will return the http.Transport instance |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no longer needed
// setMinimumTLSVersion sets the minimum TLS version required by the client to TLS v1.2 | ||
func setMinimumTLSVersion(client *http.Client) { | ||
tr := getClientTransportForSSL(client) | ||
if tr != nil { | ||
if tr, ok := client.Transport.(*http.Transport); tr != nil && ok { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we will now only call this method on an actual (non-retryable) http.Client instance
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left a few comments, but looks good overall!
This commit includes changes to the BaseService functionality so that enabling automatic retries will not cause the loss of any existing http.Client instance that might be set on the BaseService. A user can now use BaseService.SetHTTPClient() to set a custom client instance with or without retries being enabled. Also, the BaseService.GetHTTPClient() method was introduced which will return the correct http.Client instance regardless of whether retries are enabled or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! 👍
## [5.9.4](v5.9.3...v5.9.4) (2022-03-22) ### Bug Fixes * retain http.Client config when retries are enabled ([#157](#157)) ([fe093da](fe093da))
🎉 This PR is included in version 5.9.4 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
This commit includes changes to the BaseService functionality
so that enabling automatic retries will not cause the loss of
any existing http.Client instance that might be set on the
BaseService.
A user can now use BaseService.SetHTTPClient() to set
a custom client instance with or without retries being enabled.
Also, the BaseService.GetHTTPClient() method was introduced
which will return the correct http.Client instance regardless
of whether retries are enabled or not.