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

inspect: rename ImageID go field to Image #4195

Merged
merged 1 commit into from
Oct 15, 2019

Conversation

vrothberg
Copy link
Member

The json field is called Image while the go field is called ImageID,
tricking users into filtering for Image which ultimately results in an
error. Hence, rename the field to Image to align json and go.

To prevent podman users from regressing, rename Image to ImageID in
the specified filters. Add tests to prevent us from regressing. Note
that consumers of the go API that are using ImageID are regressing;
ultimately we consider it to be a bug fix.

Fixes: #4193
Signed-off-by: Valentin Rothberg [email protected]

@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: vrothberg

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

The pull request process is described 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

@openshift-ci-robot openshift-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/S labels Oct 4, 2019
@mheon
Copy link
Member

mheon commented Oct 4, 2019

LGTM

@vrothberg
Copy link
Member Author

Rerunning tests.

@vrothberg
Copy link
Member Author

Found some cycles to look into the test failures which are fixed now.

@vrothberg
Copy link
Member Author

@baude @TomSweeneyRedHat @rhatdan PTAL

@rhatdan
Copy link
Member

rhatdan commented Oct 14, 2019

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 14, 2019
@vrothberg
Copy link
Member Author

Looks like the gating job is stuck.

The json field is called `Image` while the go field is called `ImageID`,
tricking users into filtering for `Image` which ultimately results in an
error.  Hence, rename the field to `Image` to align json and go.

To prevent podman users from regressing, rename `Image` to `ImageID` in
the specified filters.  Add tests to prevent us from regressing.  Note
that consumers of the go API that are using `ImageID` are regressing;
ultimately we consider it to be a bug fix.

Fixes: containers#4193
Signed-off-by: Valentin Rothberg <[email protected]>
@openshift-ci-robot openshift-ci-robot removed the lgtm Indicates that a PR is ready to be merged. label Oct 15, 2019
@vrothberg
Copy link
Member Author

Repushed to retrigger the blocked gating test.

@vrothberg
Copy link
Member Author

Needs another lgtm :^)

@vrothberg
Copy link
Member Author

Suffering from the --rm flakes. Restarted.

@vrothberg
Copy link
Member Author

@rhatdan @TomSweeneyRedHat mind (re)dropping the final lgtm for merging?

Copy link
Member

@TomSweeneyRedHat TomSweeneyRedHat left a comment

Choose a reason for hiding this comment

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

LGTM
But I'll note that I'd personally prefer it to be ImageID everywhere. However, Docker has it as "Image" also, so probably best to be Image

@mheon
Copy link
Member

mheon commented Oct 15, 2019

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Oct 15, 2019
@openshift-merge-robot openshift-merge-robot merged commit 5f72e6e into containers:master Oct 15, 2019
sebastian-philipp added a commit to sebastian-philipp/ceph that referenced this pull request Jan 13, 2020
sebastian-philipp added a commit to sebastian-philipp/ceph that referenced this pull request Jan 20, 2020
sebastian-philipp added a commit to sebastian-philipp/ceph that referenced this pull request Jan 21, 2020
mgfritch added a commit to mgfritch/ceph that referenced this pull request Jan 22, 2020
@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 26, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inspect fails for .Image field
6 participants