Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

types: add missing log type enums #3025

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

milas
Copy link
Member

@milas milas commented Jul 29, 2022

See https://docs.docker.com/engine/api/v1.41/#tag/Container/operation/ContainerCreate.

Closes docker#2849.

Signed-off-by: Milas Bowman <milas.bowman@docker.com>
Co-authored-by: Marty Berryman <mberryman@sewerai.com>
@@ -15,9 +15,12 @@ class LogConfigTypesEnum:
'journald',
'gelf',
'fluentd',
'awslogs',
'splunk',
'etwlogs',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if relevant for how it's used, but the list of available logging drivers may depend on the daemon; static binaries may omit some (IIRC), as well as differences between windows and linux daemons. In addition, additional logging drivers can be installed as plugins.

The list of logging drivers can also be found in the /info endpoint (but the /info endpoint itself can be somewhat "heavy" as it collects info from many bits the daemon manages, including container states etc.

curl -s --unix-socket /var/run/docker.sock 'http://localhost/info' | jq .Plugins.Log
[
  "awslogs",
  "fluentd",
  "gcplogs",
  "gelf",
  "journald",
  "json-file",
  "local",
  "logentries",
  "splunk",
  "syslog"
]

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Basically; it's ok to have these for convenience, but it may be good to leave actual validation to the daemon API.

Hm... and just when I suggest that; I think there's something to look into there, as it seems like doing so takes a long time (some timeout hit after 15 seconds to look up the (plugin) driver?);

time docker create --name foo --log-driver=nosuchdriver alpine
Error response from daemon: error looking up logging plugin nosuchdriver: plugin "nosuchdriver" not found

real	0m15.048s
user	0m0.014s
sys	0m0.027s
Error response from daemon: error looking up logging plugin nosuchdriver: plugin "nosuchdriver" not found

Let me see if there's a ticket for that problem.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, that's good to know! There's no new validation here, but it was already checking to see if it was one of the enum entries 🙈

I'll probably leave that as-is for now but look at opening a follow-up PR that either just removes that validation or makes the log type field a typing.Union[LogConfigTypesEnum, str, None] so that a string can be passed for other types such as from plugins.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, yes that str makes sense (I'm not much of a Python coder, so never know what to suggest for these things 😂 )

@milas milas added this to the 6.0.0 milestone Jul 30, 2022
@milas milas modified the milestones: 6.0.0, 6.1.0 Aug 18, 2022
@milas milas removed this from the 6.x-next milestone Nov 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants