Skip to content

Remove special-case for one ":" from PullCommand#38

Merged
marcuslinke merged 2 commits intodocker-java:masterfrom
hmrm:master
Aug 7, 2014
Merged

Remove special-case for one ":" from PullCommand#38
marcuslinke merged 2 commits intodocker-java:masterfrom
hmrm:master

Conversation

@hmrm
Copy link
Copy Markdown
Contributor

@hmrm hmrm commented Aug 6, 2014

This special treatment breaks images with registries with a port
specified, e.g. my-docker-registry:5000/my-namespace/my-image

This special treatment breaks images with registries with a port
specified, e.g. my-docker-registry:5000/my-namespace/my-image
@hmrm
Copy link
Copy Markdown
Contributor Author

hmrm commented Aug 6, 2014

I haven't managed to get the test suite working yet, so thing may break tests. I'll update once I've got that working.

@hmrm
Copy link
Copy Markdown
Contributor Author

hmrm commented Aug 6, 2014

I'm seeing a bunch of failed tests even without this patch, here are the results from d6efb1b:

Failed tests:
testAddAndCopySubstitution(com.github.dockerjava.client.command.BuildImageCmdTest):
testNetCatDockerfileBuilder(com.github.dockerjava.client.command.BuildImageCmdTest): >
afterMethod(com.github.dockerjava.client.command.BuildImageCmdTest): imageId was not specified
testNginxDockerfileBuilder(com.github.dockerjava.client.command.BuildImageCmdTest):
commit(com.github.dockerjava.client.command.CommitCmdTest):
copyFromContainer(com.github.dockerjava.client.command.CopyFileFromContainerCmdTest): The file was not copied from the container. expected: but was:
createContainerWithEnv(com.github.dockerjava.client.command.CreateContainerCmdTest):
createContainerWithHostname(com.github.dockerjava.client.command.CreateContainerCmdTest):
startContainer(com.github.dockerjava.client.command.StartContainerCmdTest):
startContainerWithLinking(com.github.dockerjava.client.command.StartContainerCmdTest):

@marcuslinke
Copy link
Copy Markdown
Contributor

Hi @hmrm, what error messages you get. Here it is running without any error.

@marcuslinke
Copy link
Copy Markdown
Contributor

Oh. Now i see that your comment contains error messages. Sorry, my fault. Will investigate...

@hmrm
Copy link
Copy Markdown
Contributor Author

hmrm commented Aug 6, 2014

No worries, let me know if there's more information that would be useful!

@marcuslinke
Copy link
Copy Markdown
Contributor

What docker server version do you use for tests? The tests passes with 1.1.0 here. With latest docker version 1.1.2 i also get some test errors but these are different from yours. I have committed the fixes, so please test again now against 1.1.2 if possible.

@hmrm
Copy link
Copy Markdown
Contributor Author

hmrm commented Aug 7, 2014

Mystery solved, it looks like I'm running with an old version of docker:

Server version: 0.11.1-dev
Server API version: 1.12
Go version (server): go1.2.2
Git commit (server): f00c2e5

I'll update and try again.

@hmrm
Copy link
Copy Markdown
Contributor Author

hmrm commented Aug 7, 2014

Confirmed that this branch passes tests!

marcuslinke added a commit that referenced this pull request Aug 7, 2014
Remove special-case for one ":" from PullCommand
@marcuslinke marcuslinke merged commit c4069fc into docker-java:master Aug 7, 2014
@marcuslinke
Copy link
Copy Markdown
Contributor

@hmrm Glad that we could solve it. Thanks for your commit!

@hmrm
Copy link
Copy Markdown
Contributor Author

hmrm commented Aug 7, 2014

@marcuslinke Thanks for the near-instant response time!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants