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

ref(oci/client): update unpack_archive_layer to take cache; make pub #2523

Merged
merged 1 commit into from
May 24, 2024

Conversation

vdice
Copy link
Member

@vdice vdice commented May 22, 2024

Refactor for enabling (re-)use of the unpack_archive_layer logic (immediate use case: containerd-shim-spin)

  • Removes unpack_archive_layer from the oci Client struct and upates its signature to take a cache reference, so it can be utilized without needing to create or use an oci Client.

@itowlson
Copy link
Contributor

The archive layer stuff seems like it's OCI-specific rather than cache-specific to me. (It even has layer in the function name.) I worry that this PR moves several things to slightly awkward places and that is my job, such as functions used only by OCI moving into the common crate not because they are common but because moving the unpack function forces them to go somewhere else. Plus I like the cache being a reasonably naive thing that just stores things by content address.

I'm not very au fait with the goal here, but I'd like to see the function move back to the OCI crate. Surely it could be a free function with no dependency on the OCI Client type?

@vdice
Copy link
Member Author

vdice commented May 23, 2024

Thanks @itowlson. Indeed, it sounds like the first approach I pursued may be preferred (all kept in the oci crate, method now takes a cache reference). When you get a moment, could you take a gander at d046a26 and let me know if this approach would be more amenable? If that's a better way (or start), I can just axe the second commit.

@itowlson
Copy link
Contributor

@vdice I like that much better (although others may disagree of course).

Signed-off-by: Vaughn Dice <vaughn.dice@fermyon.com>
@vdice vdice force-pushed the ref/pub-unpack-archive-layer branch from ba2c09d to 3507f88 Compare May 23, 2024 20:24
@vdice vdice changed the title Ref(*): unpack archive layer ref(oci/client): update unpack_archive_layer to take cache; make pub May 23, 2024
@vdice vdice merged commit 9457de5 into fermyon:main May 24, 2024
17 checks passed
@vdice vdice deleted the ref/pub-unpack-archive-layer branch May 24, 2024 20:40
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