Skip to content

Conversation

@nalind
Copy link
Member

@nalind nalind commented Mar 5, 2019

Drop our dependency on the image library's manifest package by requiring that callers pass its Digest() function to us as a callback. This makes our CLI test/diagnostic tool calculate digests of s1 manifests incorrectly, but that's not something that we were testing.

@nalind
Copy link
Member Author

nalind commented Mar 5, 2019

This is going to require github.com/containers/image/storage to start passing manifest.Digest to us in several places, and CRI-O to do the same in one place.

@rhatdan
Copy link
Member

rhatdan commented Mar 5, 2019

LGTM.
Need @vrothberg and @mtrmac To approve.

@vrothberg
Copy link
Member

LGTM

@vrothberg
Copy link
Member

(FYI @siretart)

Copy link
Collaborator

@mtrmac mtrmac left a comment

Choose a reason for hiding this comment

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

Sure, this is a reasonable way to do it.

Comments are non-blocking, though do consider at least the BigDataSize API, please.

nalind added 4 commits March 6, 2019 15:03
Remove compatibility hacks for older versions which didn't track size or
digest information for big data items, hopefully without any impact.

Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Drop our dependency on the image library's manifest package by requiring
that callers pass its Digest() function to us as a callback.  This makes
our CLI test/diagnostic tool calculate digests of s1 manifests
incorrectly, but that's not something that we were testing.

Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Signed-off-by: Nalin Dahyabhai <nalin@redhat.com>
Copy link
Collaborator

@mtrmac mtrmac left a comment

Choose a reason for hiding this comment

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

LGTM

@rhatdan
Copy link
Member

rhatdan commented Mar 7, 2019

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants