Skip to content

Fix requests for docker host ending with slash#34487

Merged
vieux merged 1 commit intomoby:masterfrom
tonistiigi:host-suffix-fix
Aug 14, 2017
Merged

Fix requests for docker host ending with slash#34487
vieux merged 1 commit intomoby:masterfrom
tonistiigi:host-suffix-fix

Conversation

@tonistiigi
Copy link
Copy Markdown
Member

@tonistiigi tonistiigi commented Aug 11, 2017

relates to docker-library/docker#71

DOCKER_HOST ending with / would cause the first component of a request to be skipped. Resulting invalid ping results and skipping manually set version.

@dnephin @thaJeztah

Signed-off-by: Tonis Tiigi tonistiigi@gmail.com

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Copy link
Copy Markdown
Member

@dnephin dnephin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change looks good, should be easy to unit test as well

@aaronlehmann
Copy link
Copy Markdown

LGTM

Copy link
Copy Markdown
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@thaJeztah
Copy link
Copy Markdown
Member

do we want a test added?

Copy link
Copy Markdown
Member

@vdemeester vdemeester left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🦁

@thaJeztah
Copy link
Copy Markdown
Member

changed "fixes" to "relates to", because this needs to be vendored into docker/cli before it actually fixes the issue 😄

ping @dnephin for merging (we weren't sure if you wanted a unit test as part of this PR)

@dnephin
Copy link
Copy Markdown
Member

dnephin commented Aug 14, 2017

It seems like it would be easy to add in this PR, but if we need to merge this for some deadline I guess a follow up is fine too.

@vieux
Copy link
Copy Markdown
Contributor

vieux commented Aug 14, 2017

LGTM @tonistiigi can you open the pr in docker/cli?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants