Add Docker Model Runner documentation and compatibility tests#190
Add Docker Model Runner documentation and compatibility tests#190pradhyum6144 wants to merge 4 commits intomodelpack:mainfrom
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the ModelPack ecosystem by providing detailed documentation for integrating ModelPack models with Docker Model Runner, enabling users to easily pull and run AI models from OCI registries. Concurrently, it introduces a suite of compatibility tests to safeguard against breaking changes in the ModelPack schema, ensuring seamless interoperability for tools that rely on its structure. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces comprehensive documentation for using Docker Model Runner with ModelPack and adds a valuable suite of downstream compatibility tests. The documentation is well-structured, and the tests are thorough, ensuring that future changes won't accidentally break compatibility with consumers like Docker. I have one suggestion to improve the clarity of the field mapping documentation.
docs/docker-model-runner.md
Outdated
| | ModelPack Field | Docker Field | Description | | ||
| |---|---|---| | ||
| | `descriptor.createdAt` | `created` | Model creation timestamp | | ||
| | `descriptor.name` | `descriptor.name` | Model name | | ||
| | `descriptor.family` | `descriptor.family` | Model family | | ||
| | `descriptor.description` | `descriptor.description` | Model description | | ||
| | `descriptor.licenses` | `descriptor.licenses` | License information | | ||
| | `config.paramSize` | `parameters` | Model parameter count | | ||
| | `config.format` | `config.format` | Model format (e.g., GGUF) | | ||
| | `config.quantization` | `config.quantization` | Quantization method | | ||
| | `config.architecture` | `config.architecture` | Model architecture | | ||
| | `modelfs` | `rootfs` | Layer content addresses | |
There was a problem hiding this comment.
The field mapping table is a bit ambiguous. While some fields show a clear mapping (e.g., descriptor.createdAt -> created), others seem to map to themselves (e.g., descriptor.name -> descriptor.name). This makes it difficult to understand the final structure of the object in Docker's internal format.
To improve clarity, could you please revise the table to better illustrate the transformation? For example:
- Does
descriptor.nameremain within adescriptorobject in Docker's format? - Is
created(fromdescriptor.createdAt) a top-level field, or is it within adescriptorobject (e.g.,descriptor.created)?
A clearer representation of the final structure would greatly help users understand the mapping.
Add documentation for using Docker Model Runner with ModelPack models, including detection mechanism, field mappings, and media type conversions. Add downstream compatibility validation tests to prevent accidental breakage of fields and media types that downstream consumers depend on. Closes modelpack#151 Signed-off-by: pradhyum6144 <pradhyum314@gmail.com>
Signed-off-by: pradhyum6144 <pradhyum314@gmail.com>
Split the mapping table into renamed vs pass-through fields to make the Docker internal structure clearer per review feedback. Signed-off-by: pradhyum6144 <pradhyum314@gmail.com>
pradhyum6144
force-pushed
the
feat/docker-model-runner
branch
from
e7e2f63 to
9388d01
Compare
- Add explicit docker model pull step before run - Clarify that runtime compatibility depends on supported formats (GGUF) - Reference minimum release version (v1.0.19) instead of PR number - Tighten wording to distinguish packaging from runtime compatibility Signed-off-by: pradhyum6144 <pradhyum314@gmail.com>
@rishi-jat The compatibility validation tests are here because that's exactly what the maintainer requested. From bergwolf's comment on #151: "Add downstream users compatibility validation tests in model-spec to ensure we do not break backward compatibility by accident" The tests in schema/compat_test.go verify that model-spec's own field names and media type constants remain stable so downstream consumers like Docker Model Runner don't break when the spec evolves. This follows the same pattern as the existing schema/config_test.go which already lives in this repo. I'd recommend reading through the issue requirements and the actual code changes before concluding where they belong. |
|
Ah, I made mistake made opinion without seeing the real changes - my bad. Looking at it again, the overall direction makes sense. The only concern now is around scope: some of the tests seem to encode Docker Model Runner–specific assumptions, which could couple the spec to a single consumer. If we keep the assertions focused on general spec guarantees, this looks good. Thanks for the contribution. |
2 participants
Summary
docs/docker-model-runner.md— comprehensive guide for using Docker Model Runner with ModelPack models, covering detection mechanism, field mappings, media type conversions, and the OpenAI-compatible APIschema/compat_test.go— downstream compatibility validation tests (6 tests, 27 subtests) that verify JSON field names, media type constants, and round-trip serialization to prevent accidental breakage for downstream consumersdocs/getting-started.md(Model Management Tools, Choose Your Tool, Next Steps, Additional Resources)Closes #151
Test plan
go test ./schema/ -run TestDownstream -v)