feat: add more logging to packager2.Pull #3557
Conversation
Signed-off-by: Kit Patella <[email protected]>
✅ Deploy Preview for zarf-docs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAttention: Patch coverage is
🚀 New features to boost your workflow:
|
src/internal/packager2/pull.go
Outdated
| if err != nil { | ||
| return false, "", err | ||
| } | ||
| desc, err := remote.ResolveRoot(ctx) | ||
| if err != nil { | ||
| l.Error("unable to resolve oci descriptor", "os", platform.OS, "arch", platform.Architecture, "error", err) |
There was a problem hiding this comment.
The average user probably won't know what an OCI descriptor is. maybe something like "failed to pull package for platform {OS}/{ARCH}" would be more actionable.
I also think the error below is misleading. ResolveRoot is trying to get the manifest not the index.json. IMO combining this line and the below line would be the way to go for now
There was a problem hiding this comment.
I also feel like our error here is misleading, could you clarify what you mean by combining the two lines though?
There was a problem hiding this comment.
Combining these two lines
l.Error("unable to resolve oci descriptor", "os", platform.OS, "arch", platform.Architecture, "error", err)
return false, "", fmt.Errorf("could not fetch images index: %w", err)maybe something like
return false, "", fmt.Errorf("could not find package at %s with architecture %s: %w", err, src, platform.Architecture)Leaving out the platform.OS might make sense as well since from a user perspective since it's always MultiOS for Zarf packages which feels like more of an implementation detail.
There was a problem hiding this comment.
That's fair, definitely agreed on OS not being actionable info for users. I'll go ahead and take out the error log and we'll rely on the err log near main. Fixed in latest
Signed-off-by: Kit Patella <[email protected]>
Description
Previously, Pull would only report errors to users by passing nested errors up the chain from ORAS. This focused more on internal library details than actionable user behavior. This PR is a first pass at adding debugging context with metadata and some info, error, and debug logs to packager2.Pull.
Related Issue
Checklist before merging