-
Notifications
You must be signed in to change notification settings - Fork 163
Nop client for engine-api #117
Comments
@nishanttotla can you elaborate on how this nopclient is used? |
@duglin it's used here: https://github.com/docker/swarm/blob/master/cluster/engine.go The primary purpose is to void every action if an Engine goes down in the cluster. It avoids heavy locking around engine/list structs and pending operations. Without this, Swarm performance can degrade. |
I'm pretty sure we don't need that. You can probably achieve the same behavior with a custom Transport that always returns a client. Steps: 1- Implement the RoundTripper interface: type NopTrip struct {}
func (NopTrip) RoundTrip(*Request) (*Response, error) {
return nil, errors.New("nop")
} 2- Create a new HTTPClient using that transport: client := http.Client{
Transport: NotTrip{},
} 3- Create an API client with this new client: api, _ := client.NewClient("...", "", client, ...) 🎉 If this doesn't work, we should consider change the code enough to make it work. This approach is way more scalable than the nopclient because we don't have to add new methods to it every time something is added to the client. |
As a second note, Swarm should consider using the Circuit Breaker pattern rather than this hack: http://martinfowler.com/bliki/CircuitBreaker.html An implementation in Go: https://github.com/rubyist/circuitbreaker |
@calavera thanks, that sounds like a great suggestion! I'll close this issue for now, and I think we can consider moving away from the nopclient after the 1.2 release next month. |
Docker Swarm is planning to switch to using the
engine-api
, and move away fromsamalba/dockerclient
.One of the important provisions in
dockerclient
is thenopclient
functionality (https://github.com/samalba/dockerclient/tree/master/nopclient), which Swarm needs for efficiency reasons.engine-api
currently does not provide this, but it is simple to build. Is this something that would be acceptable if built intoengine-api
?The text was updated successfully, but these errors were encountered: