Seikho / dockerode-ts

Dockerode wrapped in TypeScript
1 stars 2 forks source link

The type definition of ContainerInspectInfo.NetworkSettings.Ports seems wrong #7

Open Feng90 opened 7 years ago

Feng90 commented 7 years ago

The structure of result returned from docker server is Map<portAndProtocol: string, Array<{ HostIp: string; HostPort: string; }>>

My docker server api version is 1.24.

See how it's modeled in another docker client:

https://github.com/spotify/docker-client/blob/master/src/main/java/com/spotify/docker/client/messages/NetworkSettings.java

Around line 59:

@Nullable @JsonProperty("PortMapping") public abstract ImmutableMap<String, Map<String, String>> portMapping();

Seikho commented 7 years ago

FYI: These types have been published as @types/dockerode now. I'll update the readme to reflect that change.

Back on topic: At the time I wrote these, the documentation (v1.24) was very different. The documentation was only examples and the example said null 😕. So I probably queried the API myself and used that. The current type ({ HostIp: string, HostPort: string }) isn't in the docs anywhere at all. 🤷‍♂️

Happy to update these on DefinitelyTyped. At the time same, the entire dockerode API has been promisified. The types need to be updated to reflect that.

Feng90 commented 7 years ago

Hi, Seikho

I noticed the null in docker documentation v1.24 before I submit this issue. It seems the detail is still missing in documentation v1.25 and v1.26.

The type definition for this port mapping in @types/dockerode is same as here. I wonder why we get different results by querying the same API.

Here is my code using type cast as a workaround:

// The typescript defition is incorrect so I add type cast here interface PortMapping { HostIp: string; HostPort: string; } const portMapping = containerInfo.NetworkSettings.Ports['8888/tcp'] as any as Array<PortMapping> const v = portMapping[0].HostIp