8000 content: support manifest labeling on completed content ingests on the client by dmcgowan · Pull Request #2117 · containerd/containerd · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

content: support manifest labeling on completed content ingests on the client #2117

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

Merged
merged 1 commit into from
Feb 12, 2018

Conversation

dmcgowan
Copy link
Member
@dmcgowan dmcgowan commented Feb 9, 2018

Fix issue where manifest content must always be fetched even if it is already fully downloaded or shared locally. Simplify children label setting and platform filtering. Prevent getting a fetcher when content shared locally.

Note this makes the filtering and labeling more composable, so we could change the behavior more easily (such as if we only wanted to label content which was saved locally, the wrapping order could just be changed).

This is the first PR to support content sharing on the client side. This PR can be backported into 1.0.x to support content sharing in clients based on 1.0.x and containerd 1.1+.

@dmcgowan dmcgowan changed the title Update fetch handling content: support manifest labeling on completed content ingests on the client Feb 9, 2018
@codecov-io
Copy link
codecov-io commented Feb 9, 2018

Codecov Report

Merging #2117 into master will not change coverage.
The diff coverage is 0%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #2117   +/-   ##
=======================================
  Coverage   45.53%   45.53%           
=======================================
  Files          80       80           
  Lines        8726     8726           
=======================================
  Hits         3973     3973           
  Misses       4107     4107           
  Partials      646      646
Flag Coverage Δ
#linux 50.24% <0%> (ø) ⬆️
#windows 41.09% <0%> (ø) ⬆️
Impacted Files Coverage Δ
images/oci/exporter.go 0% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d3a7a92...944a9ad. Read the comment docs.

}

// SetChildrenLabels is a handler wrapper which sets labels for the content on
// the children returned by the handler and passes through the children.
Copy link
Member

Choose a reason for hiding this comment

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

Must follow a handler that returns the children to be labeled.

@stevvooe
Copy link
Member
stevvooe commented Feb 9, 2018

LGTM

Nit on the docs.

Copy link
Member
@estesp estesp left a comment

Choose a reason for hiding this comment

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

LGTM; can merge after the comment nit from @stevvooe

Fix issue where manifest content must always be fetched
even if it is already fully downloaded or shared locally.
Simplify children label setting and platform filtering.
Prevent getting a fetcher when content shared locally.

Signed-off-by: Derek McGowan <derek@mcgstyle.net>
@dmcgowan dmcgowan force-pushed the update-fetch-handlers branch from adc13c7 to 944a9ad Compare February 9, 2018 22:33
@dmcgowan
Copy link
Member Author
dmcgowan commented Feb 9, 2018

Updated doc, is this ok to cherry-pick to 1.0.x?

8000

@crosbymichael
Copy link
Member

LGTM

@crosbymichael crosbymichael merged commit 5471ba9 into containerd:master Feb 12, 2018
@dmcgowan
Copy link
Member Author

Waiting until after 1.0.2 to cherry pick

@dmcgowan dmcgowan added this to the 1.0.3 milestone Feb 13, 2018
@dmcgowan dmcgowan deleted the update-fetch-handlers branch June 5, 2018 18:20
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.

5 participants
0