Skip to content
This repository has been archived by the owner on Feb 1, 2021. It is now read-only.

Use http client created in Swarm #2757

Merged

Conversation

nishanttotla
Copy link
Contributor

@nishanttotla nishanttotla commented Jul 5, 2017

The changes made in #2206 to address #1879 were lost recently. This PR re-introduces those changes. There was a similar effort in #2555 but that has stalled.

This also prevents creating unnecessary extra http connections.

Signed-off-by: Nishant Totla nishanttotla@gmail.com

@nishanttotla nishanttotla added this to the 1.3.0 milestone Jul 5, 2017
@nishanttotla
Copy link
Contributor Author

Ping @allencloud

@allencloud
Copy link
Contributor

I think it seems that we vendored a new version of package samalba/dockerclient. Do we ignore that kind of information now? @nishanttotla

@nishanttotla
Copy link
Contributor Author

@allencloud see #1879. samalba/dockerclient should be removed soon, and this is a step in that direction. The reason we're changing the vendored directory temporarily is because it makes no sense to submit a PR to samalba/dockerclient at this stage, and so we should just consider this a temporary measure.

@xihan88
Copy link

xihan88 commented Jul 6, 2017

@nishanttotla A small issue related: In func HTTPClientAndScheme (cluster/engine.go), there is no need for type check since httpClient and url.Scheme are both valid in engine. If do need a check would it be SwarmClient instead of DockerClient I guess?

@nishanttotla
Copy link
Contributor Author

@xihan88 the check is needed to make sure it isn't a NopClient (which is the case when connection has been lost)

@nishanttotla
Copy link
Contributor Author

@allencloud updated request default timeout to 30 seconds.

Signed-off-by: Nishant Totla <nishanttotla@gmail.com>
@nishanttotla nishanttotla modified the milestones: 1.2.9, 1.3.0 Jul 12, 2017
@nishanttotla
Copy link
Contributor Author

cc @wsong. This PR is the last step before we can get rid of samalba/dockerclient. While I'm confident no old API requests due to that are being sent anymore, it would be great to entirely get rid of this dependency.

@nishanttotla nishanttotla merged commit e6df4fb into docker-archive:master Jul 14, 2017
@nishanttotla nishanttotla deleted the use-right-http-client branch July 14, 2017 23:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants