improvement: Use interface for the HTTP Client#85
improvement: Use interface for the HTTP Client#85deepmap-marcinr merged 4 commits intooapi-codegen:masterfrom
Conversation
Depend on an interface, rather than a concrete implementation, so as to allow for alternate implementations to be used, such as decorated clients.
|
For this to work, you would need to remove https://github.com/deepmap/oapi-codegen/blob/master/pkg/codegen/templates/client-with-responses.tmpl#L49-L89 This would be fine with me. I would suggest to at least add a ClientOption like so: // WithDoer allows overriding the default httpClient, which is
// automatically created. The Doer interface is typically implemented
// by http.Client.
func WithDoer(doer Doer) ClientOption {
return func(c *Client) error {
c.Client = doer
return nil
}
}@deepmap-marcinr wdyt? |
|
|
||
| // HTTP client with any customized settings, such as certificate chains. | ||
| Client *http.Client | ||
| Client Doer |
There was a problem hiding this comment.
Could you please fix the code-comment to match the Doer interface.
pkg/codegen/templates/client.tmpl
Outdated
| // RequestEditorFn is the function signature for the RequestEditor callback function | ||
| type RequestEditorFn func(req *http.Request, ctx context.Context) error | ||
|
|
||
| // Doer performs HTTP requests |
There was a problem hiding this comment.
I guess it would be nice to have an additional example comment and possibly an example where one mentions that http.Client implements this interface
Oh, right. I was working off an earlier version (current release) and didn't notice that was added. It appears that the options would still be compatible, albeit maybe a bit confusing, considering the options are only used when a client is not provided. Edit:
Sure. How about keeping the name |
|
Sorry for the late reply. The other options would fail on you either way. Therefore it would be better to remove them. |
To customize the HTTP client, the WithHTTPClient option can be used.
|
Looks good to me 👍 I don’t have permissions to merge. @deepmap-marcinr can help |
| golang.org/x/tools v0.0.0-20190724185037-8aa4eac1a7c1 // indirect | ||
| ) | ||
|
|
||
| go 1.13 |
There was a problem hiding this comment.
did you use anything here which requires go 1.13? I'd like to avoid shutting out users of earlier versions of Go - since there's a compile time dependency on our runtime package.
There was a problem hiding this comment.
Oh my bad. I must have committed it by mistake - go mod is quite intrusive. No reason to add a requirement on 1.13.
* improvement: Use interface for the HTTP Client Depend on an interface, rather than a concrete implementation, so as to allow for alternate implementations to be used, such as decorated clients. * Update comments regarding the Doer interface * Update WithHTTPClient to accept a Doer implementation * Remove HTTP client configuration options To customize the HTTP client, the WithHTTPClient option can be used.
Depend on an interface, rather than a concrete implementation, so as to
allow for alternate implementations to be used, such as decorated
clients.