-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Abstracts the RoundTripper interface and provides a default implement #1602
Conversation
8f48ae4
to
5b455a5
Compare
@erikdubbelboer PTAL, We want to use this feature, Thanks a lot! |
code style problem be resolved |
type TransportFunc func(*Request, *Response) error | ||
// RoundTripper wraps every request/response. | ||
type RoundTripper interface { | ||
RoundTrip(hc *HostClient, req *Request, resp *Response) (retry bool, err error) |
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.
It feels weird to have *HostClient
in there, why is that needed? We're going to have people who try to implement the RoundTripper by modifying something and then trying to call hc.Do
for example. Which will result in an infinite loop.
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.
It feels weird to have *HostClient in there, why is that needed?
RoundTripe
relies on hc
's fields and methods to create and release conns. If move these settings into the Transport
struct may cause forward compatibility issues.
We're going to have people who try to implement the RoundTripper by modifying something and then trying to call hc.Do for example. Which will result in an infinite loop.
The TransportWrapper
testcase shows how to implement a coustom RoundTripper
like go net
sdk.
client_test.go
Outdated
|
||
func TestClientTransportEx(t *testing.T) { | ||
var reqLog string | ||
var respLog string |
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.
Why not create TransportEx
here, change it to func (t *TransportEx) RoundTrip
and set hc.Transport = &tp
so you don't have to use string pointers?
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 have rewrite the testacase. Please check again. Thanks!
Using interfaces is more extensible than using function, and it is easier to add methods to RoundTripper in the future.
…ation for enhanced extensibility (valyala#1601)
Co-authored-by: Erik Dubbelboer <erik@dubbelboer.com>
Thanks! |
close #1601