-
Notifications
You must be signed in to change notification settings - Fork 180
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
feat: support push a blob to a remote registry #489
Conversation
Please mark the command |
377b2e1
to
5e0ae90
Compare
1601d0e
to
05040c7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The overall implementation looks good.
Signed-off-by: Zoey Li <zoeyli@microsoft.com>
Signed-off-by: Zoey Li <zoeyli@microsoft.com>
Signed-off-by: Zoey Li <zoeyli@microsoft.com>
Signed-off-by: Zoey Li <zoeyli@microsoft.com>
Signed-off-by: Zoey Li <zoeyli@microsoft.com>
Signed-off-by: Zoey Li <zoeyli@microsoft.com>
Signed-off-by: Zoey Li <zoeyli@microsoft.com>
Signed-off-by: Zoey Li <zoeyli@microsoft.com>
Signed-off-by: Zoey Li <zoeyli@microsoft.com>
Signed-off-by: Zoey Li <zoeyli@microsoft.com>
Signed-off-by: Zoey Li <zoeyli@microsoft.com>
Signed-off-by: Zoey Li <zoeyli@microsoft.com>
} | ||
|
||
cmd.Flags().Int64VarP(&opts.size, "size", "", 0, "provide the blob size") | ||
cmd.Flags().StringVarP(&opts.mediaType, "media-type", "", ocispec.MediaTypeImageLayer, "specify the returned media type in the descriptor if `--descriptor` is used") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@FeynmanZhou @yizha1 What's the default media type for blobs? Currently, it is ocispec.MediaTypeImageLayer
, which is consistent with oras push
. We need to review it later.
Signed-off-by: Zoey Li <zoeyli@microsoft.com>
Signed-off-by: Zoey Li <zoeyli@microsoft.com>
Signed-off-by: Zoey Li <zoeyli@microsoft.com>
Signed-off-by: Zoey Li <zoeyli@microsoft.com>
internal/file/file.go
Outdated
// prepares the content descriptor from stdin | ||
if path == "-" { | ||
// throw err if size or digest is not provided. | ||
if size < 0 || dgst == "" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand that pushing a manifest doesn't require users to input its digest and size, because manifest has a small size and can be loaded into memory.
Preparing the content of the manifest can share the same part of reading from the file and generate the descriptor. After an offline discussion, I'll try to write a separate PrepareManifestContent()
with unit tests.
Signed-off-by: Zoey Li <zoeyli@microsoft.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Signed-off-by: Zoey Li <zoeyli@microsoft.com>
Add a command to push a blob to a remote registry
Resolves: #476
Signed-off-by: Zoey Li zoeyli@microsoft.com