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

Why are all informations wrapped in an Option in ContainerSummary? #269

Open
ClementNerma opened this issue Oct 19, 2022 · 3 comments
Open

Comments

@ClementNerma
Copy link

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?

@fussybeaver
Copy link
Owner

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.

@ClementNerma
Copy link
Author

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.

@fussybeaver
Copy link
Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants