Remove experimental from service logs#32462
Conversation
vdemeester
requested review from
aaronlehmann,
aluzzardi,
dnephin,
thaJeztah and
vdemeester
cli/command/service/logs.go
Outdated
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
thaJeztah
left a comment
thaJeztah
left a comment
There was a problem hiding this comment.
minor nits, but LGTM otherwise
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)
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 <drew.erny@docker.com>
dperny
force-pushed
the
service-logs-general-availability
branch
from
f30e95e to
306cfec
Compare
|
@thaJeztah alphabetized |
|
|
|
/cc @albers @sdurrheimer 🥇 |
|
|
||
| ```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
8 participants
Signed-off-by: Drew Erny drew.erny@docker.com
- 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.