Add LoadTarManifest to get the manifest from docker-archive#309
Add LoadTarManifest to get the manifest from docker-archive#309runcom merged 1 commit intocontainers:masterfrom
Conversation
|
@rhatdan PTAL |
|
We are going to need similar functionality to this to get the tags out of the oci image spec. |
mtrmac
left a comment
mtrmac
left a comment
There was a problem hiding this comment.
Just adding the method to Source is a much better than my proposal of splitting Source into two.
Still, loadTarManifest should be split into two: “just load everything” and “load + check that there is only one manifest”.
| return nil, errors.Wrap(err, "Error decoding tar manifest.json") | ||
| } | ||
| if len(items) != 1 { | ||
| return nil, errors.Errorf("Unexpected tar manifest.json: expected 1 item, got %d", len(items)) |
There was a problem hiding this comment.
This check should remain in place when using Source as a types.ImageSource backend.
There was a problem hiding this comment.
Can we move it to the other function? The only one that calls this? Then we would only have one loadmanafest function.
There was a problem hiding this comment.
@mtrmac I added the error check in ensureCachedDataIsPresent() after loadTarManifest() is being called.
| } | ||
|
|
||
| // LoadTarManifest loads and decodes the manifest.json | ||
| func (s *Source) LoadTarManifest() ([]manifestItem, error) { |
There was a problem hiding this comment.
Is it acceptable to return a private type from a public function? AFAIK the caller won’t be able to refer to the type in parameter/return value declarations.
(If it works for you, fine; just make sure that it does.)
There was a problem hiding this comment.
It is working in her current code.
There was a problem hiding this comment.
Yup it works fine. I looked it up and a private type can be accessed from an exported function, but the private type can't be created outside its own package.
|
👍 |
Need it in kpod pull and kpod load. These commands need all the manifests, so took out the length checking block from loadTarManifest. Signed-off-by: umohnani8 <umohnani@redhat.com>
|
@runcom PTAL |
|
LGTM |
4 participants
Need it in kpod pull and kpod load. These commands need all the manifests, so took out the length checking block from loadTarManifest.
Signed-off-by: umohnani8 umohnani@redhat.com