X Tutup
Skip to content

Support "since" field for logs command.#320

Merged
marcuslinke merged 2 commits intodocker-java:masterfrom
yongxing:master
Sep 17, 2015
Merged

Support "since" field for logs command.#320
marcuslinke merged 2 commits intodocker-java:masterfrom
yongxing:master

Conversation

@yongxing
Copy link
Copy Markdown

No description provided.

@yongxing
Copy link
Copy Markdown
Author

Looking at the check failures, it doesn't seem they were caused by this particular change.
e.g.
com.github.dockerjava.api.DockerClientException: Could not push image: Error pushing to registry: Authentication is required.

@marcuslinke
Copy link
Copy Markdown
Contributor

No worries about the failing tests. But I would like to ask for a test case for your PR.

@yongxing
Copy link
Copy Markdown
Author

absolutely, I don't know why the I didn't find the existing LogContainerCmdImplTest before I submitted this PR...

@yongxing
Copy link
Copy Markdown
Author

Added the test, here is the output from the test when I run the test locally:

23:15:35.288 DEBUG c.g.d.jaxrs.DockerCmdExecFactoryImpl - 19 * Sending client request on thread pool-72-thread-1
19 > GET https://192.168.99.100:2376/containers/2db7dc40e772de71a074153124b8c163411e0c603963f828d0d4d29f8fe40c87/logs?timestamps=0&stdout=1&stderr=1&follow=0&since=1441952195&tail=all

23:15:35.289 DEBUG o.a.h.i.c.PoolingHttpClientConnectionManager - Connection request: [route: {s}->https://192.168.99.100:2376][total kept alive: 2; route allocated: 2 of 2; total allocated: 2 of 20]
23:15:35.289 DEBUG o.a.h.i.c.DefaultManagedHttpClientConnection - http-outgoing-285: Close connection
23:15:35.289 DEBUG o.a.h.i.c.PoolingHttpClientConnectionManager - Connection leased: [id: 287][route: {s}->https://192.168.99.100:2376][total kept alive: 1; route allocated: 2 of 2; total allocated: 2 of 20]
23:15:35.339 DEBUG o.a.h.i.c.PoolingHttpClientConnectionManager - Connection [id: 287][route: {s}->https://192.168.99.100:2376][state: O=yongxing] can be kept alive indefinitely
23:15:35.339 DEBUG o.a.h.i.c.PoolingHttpClientConnectionManager - Connection released: [id: 287][route: {s}->https://192.168.99.100:2376][state: O=yongxing][total kept alive: 2; route allocated: 2 of 2; total allocated: 2 of 20]
23:15:35.339 DEBUG c.g.d.jaxrs.DockerCmdExecFactoryImpl - 20 * Client response received on thread pool-72-thread-1
20 < 200
20 < Content-Length: -1
20 < Content-Type: application/json; charset=utf-8

@yongxing
Copy link
Copy Markdown
Author

Seems like "since" field is introduced in the remote API 1.19.
https://docs.docker.com/reference/api/docker_remote_api_v1.19/#get-container-logs

I didn't find the "since" in API 1.18
https://docs.docker.com/reference/api/docker_remote_api_v1.18/#get-container-logs

The test for "since" pass on my local machine, where it has:
{"Version":"1.8.1","ApiVersion":"1.20","GitCommit":"d12ea79","GoVersion":"go1.4.2","Os":"linux","Arch":"amd64","KernelVersion":"4.0.9-boot2docker","BuildTime":"Thu Aug 13 02:49:29 UTC 2015"}

For the failed test on ci/circleci, it has:
{"ApiVersion":"1.18","Arch":"amd64","GitCommit":"2f3236d","GoVersion":"go1.4.2","KernelVersion":"3.13.0-61-generic","Os":"linux","Version":"1.6.2-circleci"}

The API version difference explains why. Also worth mentioning that adding the since field is a no-op for apiVersion < 1.19.

How do you suggest handling the dependency?

@marcuslinke
Copy link
Copy Markdown
Contributor

As stated in the README.md the current docker-java release version supports docker remote API > v1.19, so this will be no problem. As CircleCI uses a quite old docker version I will disable it soon.

@yongxing
Copy link
Copy Markdown
Author

Great! Let me know if there is anything else I need to do to get it to a mergable state from your point of view.

@marcuslinke marcuslinke merged commit 2625bca into docker-java:master Sep 17, 2015
@marcuslinke
Copy link
Copy Markdown
Contributor

Thanks for your contribution!

@marcuslinke marcuslinke changed the title This add the support for "since" field for logs command. Support "since" field for logs command. Sep 17, 2015
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

X Tutup