bollard icon indicating copy to clipboard operation
bollard copied to clipboard

Why are all informations wrapped in an `Option` in `ContainerSummary`?

Open ClementNerma opened this issue 2 years ago • 3 comments

I used the Docker::list_containers method to list all existing containers, which returns a list of ContainerSummary values.

But every field of that struct is wrapped in an Option! For instance, even the container's ID is declared as id: Option<String>. Why is that?

I'm curious to know the reason why we need to unwrap these values every time, isn't the underlying socket method guaranteed to return some informations like the ID?

ClementNerma avatar Oct 19 '22 10:10 ClementNerma

The majority of the models are generated from the upstream moby project, which releases openapi version 2.0 swagger schema against the API that is released.

So, for your model (ContainerSummary) a model is generated from this yaml stub: https://github.com/moby/moby/blob/master/api/swagger.yaml#L4433-L4498 - all fields as defined by the Docker API are considered optional.

OpenAPI version 2 has the required key to determine whether something is required https://json-schema.org/draft/2020-12/json-schema-validation.html#name-required . I think there's a case to unwrap optionals if we see these, but at least for now we don't do that yet. Initially this library focused on stability on the API, so this wasn't implemented out of caution.

I think it'd be useful to have an implementation, perhaps initially feature flagged, that generates required fields without an optional, if anyone is interested to work on it.

fussybeaver avatar Oct 19 '22 14:10 fussybeaver

Thanks for your answer!

AFAIK (I could be wrong) the required property is only for input fields, not output. Output fields are always considered required unless the nullable descriptor is provided.

Which seems logical since it would make no sense to have no guaranteed field on a stable API.

ClementNerma avatar Oct 19 '22 14:10 ClementNerma

AFAIK nullable was introduced in openapi version 3+ ... required is a JSON spec, so it's purely validation - it should apply regardless.

fussybeaver avatar Oct 19 '22 14:10 fussybeaver