-
Notifications
You must be signed in to change notification settings - Fork 215
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
allow custom http client #27
Conversation
b761f90
to
27b1a26
Compare
f8e777a
to
a9dc05f
Compare
will update documentation once the approach looks correct |
@@ -16,4 +22,5 @@ type Client interface { | |||
|
|||
SetRetryCount(count int) | |||
SetRetrier(retrier Retriable) | |||
SetCustomHTTPClient(customHTTPClient Doer) |
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.
Is there any specific reason to accept any Doer
instead of just *http.Client
?
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.
@rShetty this is so that one can use a custom HTTPClient
here, not just the one defined in one's code, but one that is defined in any imported library etc.
This blog post talks about the advantages of accepting Doer
over *http.Client
(tl;dr composability). Also, I have seen packages use Doer
to accept custom clients like fetchbot
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.
👍
http_client_test.go
Outdated
|
||
func (c *myHTTPClient) Do(request *http.Request) (*http.Response, error) { | ||
request.Header.Set("foo", "bar") | ||
response, err := c.client.Do(request) |
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.
You can just do return c.client.Do(request)
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.
yup, made the change 👍
@darshanime Thanks for this commit, have added a couple of questions to the PR. Rest all looks good to me |
@darshanime This looks good, can we go ahead and add a section in documentation of the usage. And also an example in examples folder |
@rShetty please let me know if the documentation looks okay! |
@darshanime Docs look fine. Thanks for the PR. Merging |
this pr is an attempt at allowing custom http clients (#20)
We can set a custom http client now using
SetCustomHTTPClient
The custom client needs to implement the
Doer
interface which is also implemented by thehttp.Client
in stdlib.