Skip to content

Conversation

@RishabhSaini
Copy link
Contributor

@RishabhSaini RishabhSaini commented Nov 3, 2022

This API exposes the "GetLayerInfosForCopy" API in containers/image,
which can be used to create a mapping between blob and diff IDs,
allowing pulling via diffID.

Signed-off-by: RishabhSaini rsaini@redhat.com

@RishabhSaini RishabhSaini force-pushed the issue/153 branch 2 times, most recently from d95011b to c11661c Compare November 10, 2022 21:22
Ok((fd, finish))
}

///Exposed LayerInfoForCopy
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's provide a bit more documentation, something like:

/// Returns data that can be used to find the "diffid" corresponding to a particular layer.

to start.

@cgwalters
Copy link
Collaborator

Also two other things:

  • Can you add some content into your commit messages? Even something minimal like:

Add get_layer_info_json

This API exposes the "GetLayerInfosForCopy" API in containers/image, which can be used
to create a mapping between blob and diff IDs, allowing pulling via diffID.

  • You'll need to sign off on your commit in both this repo and skopeo (the containers/ github organization has a global check)

@cgwalters
Copy link
Collaborator

And thanks so much for working on this!

@RishabhSaini RishabhSaini force-pushed the issue/153 branch 3 times, most recently from 13ecdcb to f611ecc Compare November 14, 2022 20:02
@RishabhSaini RishabhSaini marked this pull request as ready for review November 14, 2022 20:18
Copy link
Collaborator

@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

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

OK, this is looking nice and simple as is. Just one nit on the docs. That said...I'm thinking we may need to add a bit more complexity here to handle ostreedev/ostree-rs-ext#396 (comment)

We can have a realtime chat about this but basically...if we choose to go with the "dynamic detection" route, we'd need to dynamically check the protocol version the remote gives us.

See e.g. the code around let protover = semver::Version::parse(protover.as_str())?;

@RishabhSaini RishabhSaini force-pushed the issue/153 branch 2 times, most recently from 6e3cf79 to 8f9e286 Compare November 18, 2022 18:26
Cargo.toml Outdated
[features]
# See https://github.com/containers/skopeo/blob/03da797e42374892bca8759668adb0b06d087876/cmd/skopeo/proxy.go#L95
proxy_v0_2_4 = []
proxy_v0_2_5 = []
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we still need this?

@cgwalters cgwalters changed the title Fix pulling from containers-storage Add get_layer_info_json Nov 21, 2022
This API exposes the "GetLayerInfosForCopy" API in containers/image,
which can be used to create a mapping between blob and diff IDs,
allowing pulling via diffID.

Signed-off-by: RishabhSaini <rsaini@redhat.com>
Copy link
Collaborator

@cgwalters cgwalters left a comment

Choose a reason for hiding this comment

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

Thanks!

@cgwalters cgwalters enabled auto-merge November 22, 2022 01:20
@cgwalters cgwalters merged commit 356fb92 into bootc-dev:main Nov 22, 2022
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.

2 participants