swagger: define ContainerNode for GET /containers/{ID}/json#39821
swagger: define ContainerNode for GET /containers/{ID}/json#39821grooverdan wants to merge 1 commit intomoby:masterfrom grooverdan:containers_json_with_node
Conversation
Signed-off-by: Daniel Black <[email protected]>
thaJeztah
left a comment
thaJeztah
left a comment
There was a problem hiding this comment.
thanks! Sorry for the long delay; I left some suggestions / comments inline below
| type: "string" | ||
| IP: | ||
| description: "IP Address" | ||
| type: "string" |
There was a problem hiding this comment.
Can you add a x-go-name field (possibly we can generate the Go type from the swagger, but I think that needs a bug fix in the swagger generator; still can't hurt to have it in place already)
type: "string"
x-go-name: "IPAddress"|
|
||
| ContainerNode: | ||
| type: "object" | ||
| description: "Node container is running on in Swarm" |
There was a problem hiding this comment.
Could you use the description that's currently used on the type defined in Go?
Lines 320 to 322 in 39e6def
We should probably change Docker Swarm to Docker Classic Swarm to prevent confusion 😓
description: |
ContainerNode stores information about the node that a container
is running on. It's only available in Docker Classic SwarmThere was a problem hiding this comment.
So; the trouble is a bit with this struct, is that the Docker Engine itself will never propagate it; it's only used for Classic Swarm (which augments the Docker API); https://github.com/docker/swarm/blob/38c89321393f53deeaf3f69af596121740f06545/cluster/engine.go#L460-L468
So from that perspective, I'm a bit on the fence on this one. On the other hand; it's defined in the API, so 🤷♂
thaJeztah
added
area/api
|
I'm going to assume shortly that no other feedback is coming. Last chance @cpuguy83 and @dperny. Thanks for the review @thaJeztah, that all seems quite doable. |
|
I don't think I understand why we would add classic swarm api here. |
|
Is there a separate place the Classic Swarm API is documented? The reason for an API is to provide a decoupling of components and that can only happen if documented. I'm only filling out a TODO in |
|
Classic swarm API docs are here: https://docs.docker.com/swarm/swarm-api/ |
|
With this option already documented in https://docs.docker.com/swarm/swarm-api/, perhaps the better option would be to remove the Let me open a PR to do that |
|
opened #40340 |
4 participants
Define ContainerNode in swagger for GET /containers/{ID}/json
- A picture of a cute animal (not mandatory but encouraged)