This repository was archived by the owner on Oct 6, 2025. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 16
Add configure command to support Compose models implementation #106
Merged
Merged
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
c3a000b
Add configure command to support Compose models implementation
xenoscopic 935b699
configure: validate exactly one model argument before -- separator
doringeman 5b4b22b
configure: enable completion
doringeman f07d9f0
Finalize docker/model-runner module update
xenoscopic File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
There are no files selected for viewing
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| package commands | ||
|
|
||
| import ( | ||
| "fmt" | ||
|
|
||
| "github.com/docker/model-runner/pkg/inference/scheduling" | ||
| "github.com/spf13/cobra" | ||
| ) | ||
|
|
||
| func newConfigureCmd() *cobra.Command { | ||
| var opts scheduling.ConfigureRequest | ||
|
|
||
| c := &cobra.Command{ | ||
| Use: "configure [--context-size=<n>] MODEL [-- <runtime-flags...>]", | ||
| Short: "Configure runtime options for a model", | ||
| Args: func(cmd *cobra.Command, args []string) error { | ||
| if len(args) < 1 { | ||
| return fmt.Errorf( | ||
| "Model specification is required.\n\n" + | ||
| "See 'docker model configure --help' for more information", | ||
| ) | ||
| } | ||
| opts.Model = args[0] | ||
| opts.RuntimeFlags = args[1:] | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It won't, it's parsed and excluded automatically by |
||
| return nil | ||
| }, | ||
| RunE: func(cmd *cobra.Command, args []string) error { | ||
| return desktopClient.ConfigureBackend(opts) | ||
| }, | ||
| } | ||
|
|
||
| c.Flags().Int64Var(&opts.ContextSize, "context-size", -1, "context size (in tokens)") | ||
| return c | ||
| } | ||
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| command: docker model configure | ||
| short: Configure runtime options for a model | ||
| long: Configure runtime options for a model | ||
| usage: docker model configure [--context-size=<n>] MODEL [-- <runtime-flags...>] | ||
| pname: docker model | ||
| plink: docker_model.yaml | ||
| options: | ||
| - option: context-size | ||
| value_type: int64 | ||
| default_value: "-1" | ||
| description: context size (in tokens) | ||
| deprecated: false | ||
| hidden: false | ||
| experimental: false | ||
| experimentalcli: false | ||
| kubernetes: false | ||
| swarm: false | ||
| deprecated: false | ||
| hidden: false | ||
| experimental: false | ||
| experimentalcli: true | ||
| kubernetes: false | ||
| swarm: false | ||
|
|
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| # docker model configure | ||
|
|
||
| <!---MARKER_GEN_START--> | ||
| Configure runtime options for a model | ||
|
|
||
| ### Options | ||
|
|
||
| | Name | Type | Default | Description | | ||
| |:-----------------|:--------|:--------|:-------------------------| | ||
| | `--context-size` | `int64` | `-1` | context size (in tokens) | | ||
|
|
||
|
|
||
| <!---MARKER_GEN_END--> | ||
|
|
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
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we need
--here? As model is a required argument, everything after could be considered runtime flags (comparable todocker run IMAGE <command...>There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need it so we can pass runtime flags that start with
--so they're not treated as flags for theconfigurecommand itself.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting.
docker rundoesn't require this and I can't see any magic code for this purpose on https://github.com/docker/cli/blob/master/cli/command/container/run.go#L36There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, but the command doesn't usually contain "--" so it's parsed as a flag.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's not:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, nice! Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the
--terminator is a de facto standard (in GNUgetopt, Cobra, Argparse, etc.) indicating the end of flag parsing, let's go with it for now and then we can potentially make it optional in the future (see, e.g.,git help log, where--is recommended but optional in certain cases).This will give us the most flexibility later to evolve the command (e.g. supporting multiple model specifications), because honestly this command is being added with a little less design time than we'd ideally like.
For now though, it's the most unambiguous option and doesn't risk breaking in the future since it's a multi-decade convention.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you declare this command with this syntax this will have to stay forever, otherwise Compose integration will be broken
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm okay with it staying forever, it's been in use as a parsing convention for several decades now and it's baked into the APIs and CLIs of many libraries and tools. In
gitfor example, anything taking arbitrary pathspecs uses a--convention (because that's the GNU convention for arbitrary args). It doesn't cost anything to support it and there's no risk of support for it being removed from argument parsing libraries.If we didn't use it though, we wouldn't have any way to (in the future) support an arbitrary number of model specs (because we'd have no way of knowing when the model specs ended and the runtime flags began). So I think leaning into the standard here gives us more flexibility with the command design. Doing a clever parsing heuristic now would give us far less flexibility in the other direction.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok makes sense