fix: pulling not respecting setting #3472
Conversation
Signed-off-by: Allen Conlon <[email protected]>
✅ Deploy Preview for zarf-docs canceled.
|
src/internal/packager2/pull.go
Outdated
| @@ -84,6 +84,22 @@ func Pull(ctx context.Context, src, dir, shasum string, filter filters.Component | |||
| if err != nil && !errors.Is(err, os.ErrNotExist) { | |||
| return err | |||
| } | |||
| if strings.HasSuffix(name, ".tar") { | |||
There was a problem hiding this comment.
We want to avoid another unarchive and archive, as this can be quite costly depending on how big the package is, I've heard of 100+GB packages for example. Instead I believe this would be solved by changing the constant .data.tar.zst and the function nameFromMetadata depending on the value of .metadata.compressed
There was a problem hiding this comment.
I left some mental notes in the issue that I will bring over:
[R]eviewing the code a bit, I think that it has something to do with:
zarf/src/internal/packager2/pull.go
Line 49 in 33d8a2a
though it does lead to a bit of a chicken and egg problem as we would need to determine the file extension by checking the.metadata.uncompressedbut the code needs a local file to check...
So I guess is there a cleaner way to check the zarf.yaml of a remote artifact other then checking locally?
There was a problem hiding this comment.
Ah gotcha, yeah packages published to https will already be compressed/uncompressed correctly, whereas oci packages are not compressed as the files comprising the packages are in oci layers. I would move the nameFromMetadata functions into pullOCI and pullHTTP and have these functions take in a directory rather than complete path then name / rename accordingly.
There was a problem hiding this comment.
OK, I will give it a try over the weekend and see if that helps
There was a problem hiding this comment.
hm, doing some thinking and should the updated function look something like this?
func pullOCI(ctx context.Context, src, tmpPath, shasum string, filter filters.ComponentFilterStrategy) (bool, string, error)basically it still return the isPartial bool the error but adds a new tarPath string
There was a problem hiding this comment.
I'm not sure it is the prettiest way to go about it, but I think that I added the logic needed to determine the file type with both http(s) and oci pulls
Signed-off-by: Allen Conlon <[email protected]>
a1994sc
force-pushed
the
hotfix/pull-uncompressed
branch
from
7d41fa3 to
ee1caf0
Compare
| } | ||
| return isPartial, nil | ||
| return isPartial, tarPath, nil |
There was a problem hiding this comment.
@AustinAbro321 does this look like a the direction that you wanted to go?
Signed-off-by: Allen Conlon <[email protected]>
a1994sc
force-pushed
the
hotfix/pull-uncompressed
branch
from
aebc000 to
12d7dff
Compare
Description
Address
zarf package pullnot respecting.metadata.uncompressedpackage settingRelated Issue
Fixes #3464
Checklist before merging