Use shared artifacts at cache boundaries - #267
Draft
andrew wants to merge 2 commits into
Draft
Conversation
There was a problem hiding this comment.
Pull request overview
This PR standardizes cached and newly stored package file metadata by representing artifacts with a shared artifacts.Artifact type at cache boundaries, while keeping the stored SHA-256 hex digest format unchanged for ETags and verification.
Changes:
- Refactors
CacheResultto carry anartifacts.Artifact(digest/size/filename/media type) instead of separate fields, updating serving and mirroring logic accordingly. - Validates and converts cached artifact database rows into
artifacts.Artifactat the database boundary (GetCachedArtifact). - Updates tests and adds coverage for malformed cached metadata and malformed storage digests; adds new module dependencies.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/mirror/mirror.go | Uses shared artifact size metadata for mirror accounting. |
| internal/handler/handler.go | Moves cache/fetch paths to artifacts.Artifact for size/ETag/media type and DB updates. |
| internal/handler/handler_test.go | Updates tests to assert artifact metadata and adds malformed-metadata cases. |
| internal/handler/download_test.go | Updates seeded cached artifact hashes to use encoded digest. |
| internal/handler/container_test.go | Updates ETag assertions to use encoded digest. |
| internal/database/types.go | Changes cached-artifact shape to embed artifacts.Artifact. |
| internal/database/queries.go | Converts/validates cached artifact rows into artifacts.Artifact. |
| internal/database/database_test.go | Updates cached-artifact tests and adds row conversion/validation unit tests. |
| go.mod | Adds github.com/git-pkgs/artifacts and github.com/opencontainers/go-digest. |
| go.sum | Adds checksums for the new dependencies. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+229
to
+234
| if !row.ContentHash.Valid || row.ContentHash.String == "" { | ||
| return nil, fmt.Errorf("cached artifact content hash is missing") | ||
| } | ||
| if !row.Size.Valid { | ||
| return nil, fmt.Errorf("cached artifact size is missing") | ||
| } |
Comment on lines
+223
to
224
| tracker.bytes.Add(result.Artifact.Size) | ||
| m.logger.Info("mirrored", |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uses
artifacts.Artifactfor cached and newly stored package files. Cache rows are converted and validated at the database boundary while the stored hexadecimal SHA-256 remains unchanged. Response ETags and cache verification continue using the encoded digest, and mirror accounting reads shared size metadata.