Skip to content
This repository has been archived by the owner on Sep 28, 2022. It is now read-only.

refactor: optimize filters for oras.ExtendedCopy #3

Closed
wants to merge 6 commits into from

Conversation

Wwwsylvia
Copy link
Collaborator

@Wwwsylvia Wwwsylvia commented Sep 23, 2022

According to Open Container Initiative Distribution Specification, the Referrers API must include the artifactType and annotations of the referrers in the returned descriptor list. (Predecessors also supports this)

The descriptors MUST include an artifactType field that is set to the value of artifactType for an artifact manifest if present, or the configuration descriptor's mediaType for an image manifest. The descriptors MUST include annotations from the image or artifact manifest.

Related to oras-project#310
Signed-off-by: Lixia (Sylvia) Lei lixlei@microsoft.com

Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
@Wwwsylvia Wwwsylvia changed the title refactor: optimize extended copy filter refactor: optimize filters for oras.ExtendedCopy Sep 23, 2022
extendedcopy.go Outdated Show resolved Hide resolved
Copy link
Collaborator

@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.

We need to test this PR with real servers, or we have a good mock.

Also, I am wondering if the optimization works with other Targets like memory and OCI Targets.

extendedcopy.go Outdated
fp := opts.FindPredecessors
opts.FindPredecessors = func(ctx context.Context, src content.ReadOnlyGraphStorage, desc ocispec.Descriptor) ([]ocispec.Descriptor, error) {
var predecessors []ocispec.Descriptor
var err error
if fp == nil {
if rf, ok := src.(registry.ReferrerFinder); ok {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just a note and not related to this PR: With OCI Artifact support, a registry.Repository should always be a registry.ReferrerFinder. We will have registry.Referrers(ctx context.Context, repo Repository) ([]string, error later.

extendedcopy.go Outdated Show resolved Hide resolved
extendedcopy.go Outdated Show resolved Hide resolved
extendedcopy.go Outdated Show resolved Hide resolved
@Wwwsylvia
Copy link
Collaborator Author

We need to test this PR with real servers, or we have a good mock.

Also, I am wondering if the optimization works with other Targets like memory and OCI Targets.

In the current implementation, the annotations and the artifact type can be indexed only if they are populated in the descriptor on pushing.
To be safer, for memory/OCI/file Targets, we can parse the annotations of the predecessor's content, when the annotations are not present in the descriptor.

Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
Signed-off-by: Lixia (Sylvia) Lei <lixlei@microsoft.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants