Expose 'User' property on ExecCreateCmd (#707)#708
Expose 'User' property on ExecCreateCmd (#707)#708KostyaSha merged 2 commits intodocker-java:masterdocker-java/docker-java:masterfrom cdancy:ISSUE-707Copy head branch name to clipboard
Conversation
| String containerName = "generated_" + new SecureRandom().nextInt(); | ||
|
|
||
| CreateContainerResponse container = dockerClient.createContainerCmd("busybox").withCmd("top") | ||
| CreateContainerResponse container = dockerClient.createContainerCmd("busybox").withUser("root").withCmd("top") |
There was a problem hiding this comment.
Add user option here to show that creating the command works as expected.
There was a problem hiding this comment.
Please make separate test, root will be used by default afair.
There was a problem hiding this comment.
found, depends on when param was added.
There was a problem hiding this comment.
Fair enough. I guess my thinking was that setting the user, even though it's the default, would show that it works as expected without having to create a new test for this one option.
|
|
||
| ExecCreateCmdResponse execCreateCmdResponse = dockerClient.execCreateCmd(container.getId()) | ||
| .withAttachStdout(true).withCmd("touch", "/execStartTest.log").exec(); | ||
| .withAttachStdout(true).withCmd("touch", "/execStartTest.log").withUser("root").exec(); |
There was a problem hiding this comment.
Added User here to show that not only creating the command works but that executing with the user does as well.
| } | ||
|
|
||
| @Test(groups = "ignoreInCircleCi", expectedExceptions = NotFoundException.class) | ||
| public void execStartWithNonExistentUser() throws Exception { |
There was a problem hiding this comment.
Test to show that using a non-existent name fails.
| assertTrue(responseAsString.length() > 0); | ||
| } | ||
|
|
||
| @Test(groups = "ignoreInCircleCi", expectedExceptions = NotFoundException.class) |
There was a problem hiding this comment.
where expectedExceptions = NotFoundException.class is expected?
There was a problem hiding this comment.
Not clear because last line in test is assert so exception didn't happen?
There was a problem hiding this comment.
Things are not exactly clear here I admit and now I'm scratching my head wondering why I did things this way. Let me clean this up a bit along with the other minor changes.
There was a problem hiding this comment.
Ok so it's the below line that throws the NFE. My thinking was to remove everything below this line and keep the expectedExceptions = NotFoundException.class. This shows that creating with some non-existent-user disallows one to copy anything from the container, in this case /execStartTest.log, because it does not exist because the user itself does not exist.
InputStream response = dockerClient.copyArchiveFromContainerCmd(container.getId(), "/execStartTest.log").exec();
| private Boolean tty; | ||
|
|
||
| @JsonProperty("User") | ||
| private String user; |
There was a problem hiding this comment.
Since what api version it appeared? existed all the time?
There was a problem hiding this comment.
Merged for docker 1.7.0 which is analogous to api version 1.19.
KostyaSha
left a comment
There was a problem hiding this comment.
Api version, exception place.
|
@KostyaSha thanks! |
|
Thanks for contribution! |
…a#708) * Expose 'User' property on ExecCreateCmd (docker-java#707)
…a#708) * Expose 'User' property on ExecCreateCmd (docker-java#707)
Resolves #707
This change is