Remove experimental from service logs #32462
Conversation
vdemeester
requested review from
vdemeester,
aluzzardi,
dnephin,
thaJeztah and
aaronlehmann
| @@ -49,13 +49,14 @@ func newLogsCommand(dockerCli *command.DockerCli) *cobra.Command { | |||
| opts.target = args[0] | |||
| return runLogs(dockerCli, &opts) | |||
| }, | |||
| Tags: map[string]string{"experimental": ""}, | |||
There was a problem hiding this comment.
Ok, thinking what we should put here (bear with me, as this is still "new");
When talking to an older daemon, it should only be able to use this command if "experimental" is enabled. I guess that's handled by the daemon just returning a 404 if it's not enabled
However, it "officially" became a feature in docker 17.05 now, so should this be, like this?
Tags: map[string]string{"version": "1.29"},
Thinking of new clients talking to older daemons. It's tricky, because it's still possible that we're talking to an older daemon with experimental features enabled 😓
ping @vieux wdyt?
There was a problem hiding this comment.
Right now, if a current client talks to an older daemon with experimental enabled, the API is relatively unchanged, so the behavior might not be what you're expecting, but it will probably work.
There was a problem hiding this comment.
And by unexpected behavior, I might flags might not work or might be ignored, but there should not be panics.
There was a problem hiding this comment.
Actually, just recalled that I opened an issue for (roughly) this #28655. If we agree on that one, it should probably be blocked for older versions
dperny
force-pushed
the
service-logs-general-availability
branch
from
86f6c66
to
90ed9ff
Compare
|
Added requirement of API version 1.29 to avoid using endpoints on an experimental daemon. |
|
Don't merge yet; @dperny is doing some docs changes and a minor textual change for the |
dperny
force-pushed
the
service-logs-general-availability
branch
from
90ed9ff
to
f30e95e
Compare
|
|
||
| Options: | ||
| --details Show extra details provided to logs | ||
| --no-resolve Do not map IDs to Names in output | ||
| --no-truc Do not truncate output |
There was a problem hiding this comment.
s/--no-truc/--no-trunc/ :smile:
There was a problem hiding this comment.
also needs to be swapped with --no-task-id's (alphabetical)
| -t, --timestamps Show timestamps | ||
| --tail string Number of lines to show from the end of the logs (default "all") |
There was a problem hiding this comment.
same here; --tail comes before --timestamps in output
Service logs API is now stable. Service logs now support all features, except retrieving details provided to the log driver. Signed-off-by: Drew Erny <[email protected]>
dperny
force-pushed
the
service-logs-general-availability
branch
from
f30e95e
to
306cfec
Compare
|
@thaJeztah alphabetized |
|
|
|
/cc @albers @sdurrheimer 🥇 |
| @@ -17,14 +16,16 @@ advisory: "experimental" | |||
| # service logs | |||
|
|
|||
| ```Markdown | |||
| Usage: docker service logs [OPTIONS] SERVICE | |||
| Usage: docker service logs [OPTIONS] SERVICE|TASK | |||
There was a problem hiding this comment.
Hmmm, so for bash completion I will need a list of all tasks. ATM, this can be done with
docker service ps ""
Is this supported syntax, will this continue to work?
There was a problem hiding this comment.
Good question, looks like a "tricky" one
|
Windows timed out. |
…bility Remove experimental from service logs
Signed-off-by: Drew Erny [email protected]
- What I did
Remove experimental flag from
service logs. API is now stable. Service logs now support all features, except retrieving details provided to the log driver.- How I did it
Removed the experimental-specific code, updated the tests
- How to verify it
Tests pass
- Description for the changelog
Added ability to get logs from services and tasks, which was previously experimental.