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

dump: handle duplicates #1951

Merged

Conversation

giuseppe
Copy link
Member

@giuseppe giuseppe commented Jun 5, 2024

Add validation for duplicate entries. Duplicates are ignored, unless there is a mismatch in the values. In that case, an error is returned.

Also refactor the tests to avoid duplication.

Signed-off-by: Giuseppe Scrivano gscrivan@redhat.com

giuseppe added 2 commits June 5, 2024 13:13
move all the test cases into a struct instead of declaring each of
them separately.

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Add validation for duplicate entries.  Duplicates are ignored, unless
there is a mismatch in the values.  In that case, an error is returned.

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
Copy link
Contributor

openshift-ci bot commented Jun 5, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: giuseppe

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 openshift-ci bot added the approved label Jun 5, 2024
@cgwalters
Copy link
Contributor

This makes sense to me, but can you explain briefly what motivated the change just for reference? Did you come across an image in the wild with duplicates? Were you just thinking through the problem domain? Is this split out of a different larger pending work PR?

@rhatdan
Copy link
Member

rhatdan commented Jun 5, 2024

LGTM

@giuseppe
Copy link
Member Author

giuseppe commented Jun 5, 2024

This makes sense to me, but can you explain briefly what motivated the change just for reference? Did you come across an image in the wild with duplicates? Were you just thinking through the problem domain? Is this split out of a different larger pending work PR?

no, I've not encountered yet such case. I was just looking at ways to make the dump code more robust, since most of the issues we had with composefs were caused by the conversion to the dump format

@rhatdan
Copy link
Member

rhatdan commented Jun 5, 2024

/lgtm

@openshift-ci openshift-ci bot added the lgtm label Jun 5, 2024
@openshift-merge-bot openshift-merge-bot bot merged commit de5b13b into containers:main Jun 5, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants