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

#42818 change query param from comma separated list to exploded list … #43073

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

CosmosViewer
Copy link

@CosmosViewer CosmosViewer commented Dec 9, 2021

…in swagger

- What I did
I fixed the issue by adding the "explode: true" argument to swagger in the imageGetAll description page. This way, it will not show as a comma separated array.

- How I did it
By adding the "explode: true" argument

- How to verify it
By verifying the docs page

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

This is my first contribution, and I'm still a little confused if this is the right way to do it. I chose this issue because it seemed good to start. Please, comment and help! Thanks!

…oded list in swagger

Signed-off-by: Rafael Pili <hpcponte@gmail.com>
@thaJeztah
Copy link
Member

Thanks for contributing! I see you added a new (v1.42) file in this PR, which shouldn't be the case (the versioned files inside the docs directory correspond with snapshots of the api/swagger.yaml, specific to each release of the API). Could you instead update the api/swagger.yaml? After that we can update the versioned files in the docs with the same changes (feel free to do so in the same pull request, but as a separate commit; I'm also happy to take care of that after the changes to the api/swagger.yaml were merged).

Looking at the change itself, I did a diff between v1.41 and v1.42;

diff --git a/docs/api/v1.41.yaml b/docs/api/v1.42.yaml
index fba45b6b15..703af61552 100644
--- a/docs/api/v1.41.yaml
+++ b/docs/api/v1.42.yaml
@@ -8423,6 +8423,7 @@ paths:
           in: "query"
           description: "Image names to filter by"
           type: "array"
+          explode: true
           items:
             type: "string"
       tags: ["Image"]

I must admit that I wasn't aware of this option in the OpenAPI spec, so had to look it up (we most likely have other query-parameters that have the same issue, which would have to be updated as well in our swagger files).

Looking at the explode option, I think that option was added in OpenAPI 3.0, but the swagger/OpenAPI file in this repository is still using the 2.0 spec. Going through the documentation for OpenAPI 2.0, I think collectionFormat: multi would be the equivalent of explode; https://swagger.io/docs/specification/2-0/describing-parameters/#array

I see that one is already used for one parameter;

collectionFormat: multi

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See my comment above

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

Successfully merging this pull request may close these issues.

None yet

3 participants