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.

Sign up for GitHub

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

Jump to bottom

[No merge] swagger: add missing parameters #9126

Closed
wants to merge 1 commit into from
Closed

[No merge] swagger: add missing parameters #9126

wants to merge 1 commit into from

Conversation

tmds
Copy link
Contributor

@tmds tmds commented Jan 27, 2021

@jwhonce @baude this PR gives an overview of parameters which are missing in podman swagger compared to docker.

This PR isn't meant for merging. You can go over the parameters and see which you want to add, and which you won't support.

@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: tmds
To complete the pull request process, please assign tomsweeneyredhat after the PR has been reviewed.
You can assign the PR to them by writing /assign @tomsweeneyredhat in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@tmds
swagger: add missing parameters
d0dcdf2 
Signed-off-by: Tom Deseyn <tom.deseyn@gmail.com>
@tmds tmds force-pushed the swagger_missing_parameters branch from 87d537f to d0dcdf2 Compare January 27, 2021 15:09
@tmds
Copy link
Contributor Author

tmds commented Jan 28, 2021

@jwhonce @baude unless I misunderstand what the parameters are for, these missing parameters make it not possible to:

  • build images, and
  • create containers.

@baude
Copy link
Member

baude commented Jan 28, 2021

@jwhonce @baude unless I misunderstand what the parameters are for, these missing parameters make it not possible to:

* build images, and

* create containers.

Well that certainly cannot be true as we create containers and build images in both the compat and libpod sides of the api.

@jwhonce what do you make of this?

@mheon
Copy link
Member

mheon commented Jan 28, 2021

AFAIK: The parameters are there, but not documented, so an API client made using the swagger can't access them.

@tmds
Copy link
Contributor Author

tmds commented Feb 3, 2021

How do we proceed with this?

A suggestion: can you go over this PR, and create specific issue(s) for adding the parameters and validating they work?

I think it's important to add these parameters because without them some really basic operations are not exposed through the swagger.

@jwhonce
Copy link
Member

jwhonce commented Feb 10, 2021

Aligning the swagger to the as-built code is always good. An audit of all the endpoints will take some time. After Podman v3.0 is released let's add a card to Jira (@tmds Our Jira instance will soon be public.) to do the audit and generate issues for the missing parameters. That is the pattern we've used in the past.

Any objections?

There are some issues with this PR regarding the references, once fixed I would merge.

@tmds
Copy link
Contributor Author

tmds commented Feb 22, 2021

After Podman v3.0 is released let's add a card to Jira (@tmds Our Jira instance will soon be public.) to do the audit and generate issues for the missing parameters. That is the pattern we've used in the past.

@jwhonce now that podman 3.0 is out, can you add these cards?

@vrothberg
Copy link
Member

Friendly ping

@tmds
Copy link
Contributor Author

tmds commented Mar 16, 2021

ping @jwhonce

@tmds tmds mentioned this pull request Mar 26, 2021
Closed
@github-actions
Copy link

A friendly reminder that this PR had no activity for 30 days.

@jwhonce jwhonce removed the stale-pr label Apr 16, 2021
This was referenced Apr 16, 2021
Closed
Closed
Closed
jwhonce
jwhonce reviewed Apr 16, 2021
View reviewed changes
pkg/api/server/register_containers.go Show resolved Hide resolved
pkg/api/server/register_containers.go
Comment on lines +381 to +384
// - in: query
// name: one-shot
// type: boolean
// default: false
Copy link
Member

Choose a reason for hiding this comment

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

See #10058

pkg/api/server/register_images.go
Comment on lines +54 to +58
// - in: query
// name: platform
// description: Platform in the format os[/arch[/variant]]
// type: string
// default: ""
Copy link
Member

Choose a reason for hiding this comment

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

I would expect this to be grouped with other query parameters.

See #10060

pkg/api/server/register_images.go Show resolved Hide resolved
pkg/api/server/register_networks.go
Comment on lines +60 to +66
// - in: query
// name: verbose
// type: boolean
// default: false
// - in: query
// name: scope
// type: string
Copy link
Member

Choose a reason for hiding this comment

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

See #10061

pkg/api/server/register_containers.go Show resolved Hide resolved
jwhonce
jwhonce reviewed Apr 16, 2021
View reviewed changes
Copy link
Member

@jwhonce jwhonce left a comment

Choose a reason for hiding this comment

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

Please see issues. As they are resolved I would like to update those areas of the swagger.

@tmds I also agreed to some changes that do not require code changes rather they comment existing behavior. Would you want to provide those changes in a PR that I can get merged for you?

@github-actions
Copy link

A friendly reminder that this PR had no activity for 30 days.

@rhatdan rhatdan removed the stale-pr label May 17, 2021
@rhatdan
Copy link
Member

rhatdan commented May 17, 2021

@tmds Since this PR has not been touched in a couple of months, I am going to close, Please reopen if you want to continue to work on it.

@rhatdan rhatdan closed this May 17, 2021
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 23, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jwhonce jwhonce jwhonce left review comments

Assignees
No one assigned
Labels
locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@tmds @openshift-ci-robot @baude @mheon @jwhonce @vrothberg @rhatdan