dotnet / Docker.DotNet

:whale: .NET (C#) Client Library for Docker API
https://www.nuget.org/packages/Docker.DotNet/
MIT License
2.26k stars 380 forks source link

Timeout field of PluginEnableParameter should be a string not long? #306

Open ghost opened 6 years ago

ghost commented 6 years ago

I installed a plugin and tried to enable it. With Timeout = 0, docker throws strconv.Atoi exception saying invalid syntax. If I directly use the source code with Timeout field set to string, and passing Timeout = "0" succeeds.

jterry75 commented 6 years ago

Hmm, from what I can tell the documentation from Docker, the typedef and the handler all say int.

Could you do me a favor and post the json from running your daemon with the -D flag while using the docker.exe to enable the plugin and when using Docker.DotNet to enable the plugin? I fail to see how this should be treated as a string and just want to make sure we are fixing the right thing. The json should show us the differences in how we are serializing vs how the docker cli is.

Thanks!

ghost commented 6 years ago

Apparently, docker complains only if Timeout is set to 0 or not set at all. For non-zero integer value for Timeout, plugin enable API works fine.

jterry75 commented 6 years ago

I think this is a bug in docker. The API documentation has marked that this is an optional, integer with default value of 0. However the router and client ALWAYS specify a string therefore making this required.

I think the right fix is to make modeldefs.go Change to:

// PluginEnableParameters for POST /plugins/(name)/enable
type PluginEnableParameters struct {
    Timeout string `rest:"query,timeout,required,\"0\""`
}

Regenerate the code and that should change PluginEnableParameters.cs

jterry75 commented 6 years ago

I see your PR is still up. Did you have a chance to try this alternate way out?