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

feat: support fetch a blob from a remote registry #520

Merged
merged 9 commits into from
Sep 8, 2022

Conversation

lizMSFT
Copy link
Contributor

@lizMSFT lizMSFT commented Aug 22, 2022

support fetch a blob from a remote registry

Resolves: #475
Signed-off-by: Zoey Li [email protected]

@codecov-commenter
Copy link

codecov-commenter commented Aug 22, 2022

Codecov Report

Merging #520 (097dc7f) into main (45a1024) will increase coverage by 0.01%.
The diff coverage is 58.33%.

@@            Coverage Diff             @@
##             main     #520      +/-   ##
==========================================
+ Coverage   70.78%   70.79%   +0.01%     
==========================================
  Files          11       11              
  Lines         421      428       +7     
==========================================
+ Hits          298      303       +5     
- Misses        100      101       +1     
- Partials       23       24       +1     
Impacted Files Coverage Δ
internal/cache/target.go 58.97% <58.33%> (+1.22%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@lizMSFT lizMSFT marked this pull request as ready for review August 26, 2022 13:53
Copy link

@nima nima left a comment

Choose a reason for hiding this comment

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

Have only reviewed one file so far, but publishing in advance 😃

cmd/oras/blob/fetch.go Outdated Show resolved Hide resolved
cmd/oras/blob/fetch.go Show resolved Hide resolved
cmd/oras/blob/fetch.go Outdated Show resolved Hide resolved
cmd/oras/blob/fetch.go Show resolved Hide resolved
cmd/oras/blob/fetch.go Outdated Show resolved Hide resolved
internal/cas/fetch.go Outdated Show resolved Hide resolved
Copy link

@nima nima left a comment

Choose a reason for hiding this comment

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

Left some comments from the perspective of someone new to the project.

cmd/oras/blob/fetch.go Outdated Show resolved Hide resolved
internal/cas/fetch.go Outdated Show resolved Hide resolved
cmd/oras/blob/fetch.go Show resolved Hide resolved
cmd/oras/blob/fetch.go Outdated Show resolved Hide resolved
cmd/oras/blob/fetch.go Outdated Show resolved Hide resolved
internal/cas/fetch.go Outdated Show resolved Hide resolved
@lizMSFT lizMSFT force-pushed the liz/475 branch 13 times, most recently from 3089a08 to 097dc7f Compare September 8, 2022 03:45
@shizhMSFT shizhMSFT added this to the v0.15.0 milestone Sep 8, 2022
internal/cache/target.go Outdated Show resolved Hide resolved
internal/cache/target.go Outdated Show resolved Hide resolved
@lizMSFT lizMSFT force-pushed the liz/475 branch 2 times, most recently from defc485 to f400dd7 Compare September 8, 2022 07:26
cmd/oras/blob/fetch.go Outdated Show resolved Hide resolved
}

// fetch blob
desc, rc, err := oras.Fetch(ctx, src, opts.targetRef, oras.FetchOptions{})
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems the rc is never verified against the desc.

Copy link
Contributor Author

@lizMSFT lizMSFT Sep 8, 2022

Choose a reason for hiding this comment

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

create an issue to track this #547

cmd/oras/blob/fetch.go Outdated Show resolved Hide resolved
cmd/oras/blob/fetch.go Show resolved Hide resolved
cmd/oras/blob/fetch.go Outdated Show resolved Hide resolved
Signed-off-by: Zoey Li <[email protected]>
Signed-off-by: Zoey Li <[email protected]>
Copy link
Contributor

@shizhMSFT shizhMSFT left a comment

Choose a reason for hiding this comment

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

LGTM

@shizhMSFT shizhMSFT merged commit 9a014d6 into oras-project:main Sep 8, 2022
TerryHowe pushed a commit to TerryHowe/oras that referenced this pull request Feb 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Command to fetch a blob from a remote registry
7 participants