rpc: Support specifying HTTP client in RPC dialing#15836
Conversation
There was a problem hiding this comment.
Haha, thanks. Fixed it.
stevenroose
force-pushed
the
http-client
branch
2 times, most recently
from
0daaabf to
3eea7f9
Compare
|
ping.. |
There was a problem hiding this comment.
I don't think it's a good idea to define this interface here, as any eventual extra method (that's provided by Go's http.Client) will result in breaking the interface. Let's just directly require specifying a *http.Client. That way we're future proof.
There was a problem hiding this comment.
Yeah had that first, but it makes it impossible to use wrappers of an http.Client like proxies or monitors.
There was a problem hiding this comment.
AFAIK that's what http.Client.Transport is for:
// Transport specifies the mechanism by which individual
// HTTP requests are made.
// If nil, DefaultTransport is used.
Transport RoundTripper
You can define all proxy and monitoring methods inside the http Client and that's how other libraries use it too (e.g. Google's Go APIs googleapis/google-api-go-client#146 (comment)).
There was a problem hiding this comment.
Hmm, didn't know about the Transport field. That's a smart thing they did there. Will change, one sec.
There was a problem hiding this comment.
The usual "way" in Go code when initializing something with an extra parameter is With. I.e. DialHTTPWithClient. Please rename.
There was a problem hiding this comment.
Yeah that makes sense.
karalabe
left a comment
karalabe
left a comment
There was a problem hiding this comment.
Generally ok, just a couple minor issues.
stevenroose
force-pushed
the
http-client
branch
from
3eea7f9 to
cb0f70d
Compare
Adds a minimal interface that captures http.Client and adds a new method rpc.DialHTTPClient that takes a client using that interface. The existing rpc.DialHTTP method is then alternatively implemented by using the new rpc.DialHTTPClient method provided with a standard *http.Client.
stevenroose
force-pushed
the
http-client
branch
from
cb0f70d to
1625358
Compare
* rpc: Support specifying HTTP client in RPC dialing Adds a minimal interface that captures http.Client and adds a new method rpc.DialHTTPClient that takes a client using that interface. The existing rpc.DialHTTP method is then alternatively implemented by using the new rpc.DialHTTPClient method provided with a standard *http.Client. * rpc: fix minor doc typos
* rpc: Support specifying HTTP client in RPC dialing Adds a minimal interface that captures http.Client and adds a new method rpc.DialHTTPClient that takes a client using that interface. The existing rpc.DialHTTP method is then alternatively implemented by using the new rpc.DialHTTPClient method provided with a standard *http.Client. * rpc: fix minor doc typos
3 participants
Adds a minimal interface that captures
http.Clientand adds a new methodrpc.DialHTTPClientthat takes a client using that interface. The existingrpc.DialHTTPmethod is then alternatively implemented by using the newrpc.DialHTTPClientmethod provided with a standard*http.Client.