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

CreateImageInfo is not shown in API doc viewer #44504

Open
davidhsingyuchen opened this issue Nov 22, 2022 · 4 comments · May be fixed by #44518
Open

CreateImageInfo is not shown in API doc viewer #44504

davidhsingyuchen opened this issue Nov 22, 2022 · 4 comments · May be fixed by #44518
Labels
area/api area/docs kind/bug Bugs are bugs. The cause may or may not be known at triage time so debugging may be needed.

Comments

@davidhsingyuchen
Copy link
Contributor

davidhsingyuchen commented Nov 22, 2022

Description

Pulling an image via POST /images/create is returning CreateImageInfo when the HTTP status code is 200, but there is no corresponding schema definition in the API doc viewer.

It seems that we may need to add

          schema:
            $ref: "#/definitions/CreateImageInfo"

to

moby/api/swagger.yaml

Lines 8270 to 8271 in 8bb5815

200:
description: "no error"

and the one of the next version (i.e., v1.42).

I was going to create a PR for this, but I found that the exact issue exists for multiple objects (e.g., also PushImageInfo), and I'm not sure if there's any special reason why we're not already doing so, hence this issue.

Reproduce

Open https://docs.docker.com/engine/api/v1.41/#tag/Image/operation/ImageCreate and inspect the response of 200.

Expected behavior

See CreateImageInfo instead of an empty object.

docker version

inapplicable

docker info

inapplicable

Additional Info

No response

@davidhsingyuchen davidhsingyuchen added kind/bug Bugs are bugs. The cause may or may not be known at triage time so debugging may be needed. status/0-triage labels Nov 22, 2022
@corhere
Copy link
Contributor

corhere commented Nov 22, 2022

I was going to create a PR for this, but I found that the exact issue exists for multiple objects (e.g., also PushImageInfo), and I'm not sure if there's any special reason why we're not already doing so, hence this issue.

There is no special reason, as far as I am aware. Your help in improving the quality of the API docs would be really, really appreciated. Thank you so much!

@thaJeztah
Copy link
Member

thaJeztah commented Nov 23, 2022

Open https://docs.docker.com/engine/api/v1.41/#tag/Container/operation/ContainerCreate and inspect the response of 200.

You probably meant https://docs.docker.com/engine/api/v1.41/#tag/Image/operation/ImageCreate (I noticed the link pointed to container create, not image create 😅

I was going to create a PR for this, but I found that the exact issue exists for multiple objects (e.g., also PushImageInfo), and I'm not sure if there's any special reason why we're not already doing so, hence this issue.

My best guess why these weren't documented would be that these provide a "jsonprogress" response, which is a line-delimited stream, and possibly swagger / openAPI doesn't have a proper way to document those (an array would be the closest match, but it's not an array). I know at the time that format wasn't formalised, although there's now https://jsonlines.org, which does, so perhaps there's way to document it.

curl --unix-socket /var/run/docker.sock -s --no-buffer -X POST 'http://localhost/images/create?fromImage=hello-world&tag=latest'
{"status":"Downloading","progressDetail":{"total":2561},"progress":"[\u003e                                                  ]       0B/2.561kB","id":"faa03e786c97"}
{"status":"Downloading","progressDetail":{"total":2561},"progress":"[\u003e                                                  ]       0B/2.561kB","id":"faa03e786c97"}
{"status":"Downloading","progressDetail":{"total":2561},"progress":"[\u003e                                                  ]       0B/2.561kB","id":"faa03e786c97"}
{"status":"Downloading","progressDetail":{"total":2561},"progress":"[\u003e                                                  ]       0B/2.561kB","id":"faa03e786c97"}
{"status":"Downloading","progressDetail":{"total":2561},"progress":"[\u003e                                                  ]       0B/2.561kB","id":"faa03e786c97"}
{"status":"Downloading","progressDetail":{"total":2561},"progress":"[\u003e                                                  ]       0B/2.561kB","id":"faa03e786c97"}
{"status":"Downloading","progressDetail":{"total":2561},"progress":"[\u003e                                                  ]       0B/2.561kB","id":"faa03e786c97"}
{"status":"Downloading","progressDetail":{"total":2561},"progress":"[\u003e                                                  ]       0B/2.561kB","id":"faa03e786c97"}
{"status":"Downloading","progressDetail":{"total":2561},"progress":"[\u003e                                                  ]       0B/2.561kB","id":"faa03e786c97"}
{"status":"Download complete","progressDetail":{"hidecounts":true},"id":"faa03e786c97"}
{"status":"Downloading","progressDetail":{"total":525},"progress":"[\u003e                                                  ]       0B/525B","id":"432f982638b3"}
{"status":"Download complete","progressDetail":{"hidecounts":true},"id":"432f982638b3"}
{"status":"Downloading","progressDetail":{"total":1485},"progress":"[\u003e                                                  ]       0B/1.485kB","id":"46331d942d63"}
{"status":"Downloading","progressDetail":{"total":1485},"progress":"[\u003e                                                  ]       0B/1.485kB","id":"46331d942d63"}
{"status":"Download complete","progressDetail":{"hidecounts":true},"id":"46331d942d63"}
{"status":"Downloading","progressDetail":{"total":3208},"progress":"[\u003e                                                  ]       0B/3.208kB","id":"7050e35b49f5"}
{"status":"Downloading","progressDetail":{"total":3208},"progress":"[\u003e                                                  ]       0B/3.208kB","id":"7050e35b49f5"}
{"status":"Download complete","progressDetail":{"hidecounts":true},"id":"7050e35b49f5"}

If there is a way to document that, we definitely should. I do notice that the CreateImageInfo and PushImageInfo may need some love as well (fields are not documented, and some of those fields will be omitted when empty, so not all of them may actually be used in these responses (see the example above).

@thaJeztah
Copy link
Member

We should probably also document that (for streaming endpoints), that a 200 (success) status code will be returned if the initial request succeeded, but that failures happening during the process; i.e., if a failure happens while the image pull is in progress, or (in case of "import"), if the uploaded image is invalid; that the error is returned as part of the stream response JSON; this is because the status code cannot be updated once sent (so the 200 only indicates the request was received successfully).

@davidhsingyuchen
Copy link
Contributor Author

davidhsingyuchen commented Nov 23, 2022

Thanks a lot for the prompt responses and detailed explanations!

You probably meant https://docs.docker.com/engine/api/v1.41/#tag/Image/operation/ImageCreate (I noticed the link pointed to container create, not image create 😅

Fixed.

There is no special reason, as far as I am aware. Your help in improving the quality of the API docs would be really, really appreciated. Thank you so much!

Will create a PR to at least reference the schema, and re. documentation of individual fields:

fields are not documented, and some of those fields will be omitted when empty, so not all of them may actually be used in these responses (see the example above).

I happened to create another issue to track this: #44505.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/api area/docs kind/bug Bugs are bugs. The cause may or may not be known at triage time so debugging may be needed.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants