transport: Introduce NewHTTPTransportOptions#1168
transport: Introduce NewHTTPTransportOptions#1168marclop merged 8 commits intoelastic:masterfrom marclop:f/add-transport.NewHTTPTransportOptions
Conversation
Introduces a new `transport.NewHTTPTransportOptions()` function which accepts an Options struct to be able to customize the `httptransport` without using environment variables Signed-off-by: Marc Lopez Rubio <marc5.12@outlook.com>
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
🤖 GitHub commentsTo re-run your PR in the CI, just comment with:
|
transport/http.go
Outdated
| t.SetSecretToken(opts.SecretToken) | ||
| } | ||
| t.SetServerURL(serverURLs...) | ||
| t.SetServerURL(opts.ServerURLs...) |
There was a problem hiding this comment.
IIANM this will panic if HTTPTransportOptions.ServerURLs is empty. This should probably also return an error.
Maybe worth adding a Validate method, like:
func (o HTTPTransportOptions) Validate() error {
if len(o.ServerURLs) == 0 {
// ...
}
if len(o.ServerTimeout) < 0 {
// ...
}
return nil
}
simitt
left a comment
There was a problem hiding this comment.
In my opinion NewHTTPTransport and NewHTTPTransportOptions should deliever the same default values for non-customized options, so also ensure that the same default values for ServerURLs and ServerTimeout are set.
Can you please add a test for when using NewHTTPTransportOptions?
Signed-off-by: Marc Lopez Rubio <marc5.12@outlook.com>
Signed-off-by: Marc Lopez Rubio <marc5.12@outlook.com>
axw
left a comment
There was a problem hiding this comment.
LGTM, just a couple of minor things
Signed-off-by: Marc Lopez Rubio <marc5.12@outlook.com>
Description
Introduces a new
transport.NewHTTPTransportOptions()function whichaccepts an Options struct to be able to customize the
httptransportwithout using environment variables