test+docs: validate ModelPack compatibility in Docker Model Runner#744
test+docs: validate ModelPack compatibility in Docker Model Runner#744rishi-jat wants to merge 2 commits intodocker:mainfrom
Conversation
Signed-off-by: Rishi Jat <rishijat098@gmail.com>
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In the
pull modelpack artifacttest, comparingpulledContentandoriginalContentviastring(...) != string(...)is unnecessary for byte slices; usingbytes.Equal(pulledContent, originalContent)avoids extra allocations and handles arbitrary binary content more idiomatically.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In the `pull modelpack artifact` test, comparing `pulledContent` and `originalContent` via `string(...) != string(...)` is unnecessary for byte slices; using `bytes.Equal(pulledContent, originalContent)` avoids extra allocations and handles arbitrary binary content more idiomatically.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Code Review
This pull request adds documentation and an end-to-end test for ModelPack compatibility. The documentation changes are clear and helpful. The new test case is a good addition, but it uses a workaround that makes it not fully representative of a real-world scenario. I've added a comment with a suggestion to improve the test's accuracy, which will require a corresponding change in the validation logic.
There was a problem hiding this comment.
Pull request overview
Adds documentation and an end-to-end distribution test intended to validate pulling/running CNCF ModelPack OCI artifacts with Docker Model Runner.
Changes:
- Adds a new
TestClientPullModelsubtest that pushes and pulls a “ModelPack” artifact via the test registry and validates the pulled weights/config. - Introduces a test-only
modelPackTestArtifacthelper to construct an OCI artifact with ModelPack-style layers/config. - Adds a README section describing ModelPack media type compatibility and how to pull/run ModelPack artifacts.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pkg/distribution/distribution/client_test.go | Adds an E2E pull test and helper artifact implementation for ModelPack-like images. |
| README.md | Documents ModelPack compatibility and expected media types for config/weight layers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
/cc @bergwolf |
Signed-off-by: Rishi Jat <rishijat098@gmail.com>
Adds an end-to-end test validating the ModelPack compatibility introduced in #516.
The test verifies that a ModelPack OCI artifact can be pulled and handled by Docker Model Runner.
Also adds documentation explaining how to run ModelPack artifacts using Docker Model Runner.
Related issue: modelpack/model-spec#151