add --upgrade-all flag to refresh any stale models
#2179
Conversation
| Username string `json:"username"` | ||
| Password string `json:"password"` | ||
| Stream *bool `json:"stream,omitempty"` | ||
| CurrentDigest string `json:"current_digest,omitempty"` |
There was a problem hiding this comment.
What's the advantage of the client sending the current digest versus just re-pulling all the models by name?
There was a problem hiding this comment.
The digest is needed for the Etag which allows it to skip pulling things unnecessarily. This is what is passed with the "If-None-Match" header.
| @@ -357,6 +357,42 @@ func CopyHandler(cmd *cobra.Command, args []string) error { | |||
| } | |||
|
|
|||
| func PullHandler(cmd *cobra.Command, args []string) error { | |||
| upgradeAll, err := cmd.Flags().GetBool("upgrade-all") | |||
There was a problem hiding this comment.
I know I'm going to fall into the same trap of getting confused between upgrade and update and will end up retyping this 😆
This is a minor non-blocking gripe, but what do you think about something along these lines?
ollama pull --sync-installed
ollama pull --fetch-all
ollama pull --latestTrying to avoid the above confusion while also not introducing confusion about whether "all" refers to remote or local.
| @@ -1105,17 +1115,27 @@ func PullModel(ctx context.Context, name string, regOpts *RegistryOptions, fn fu | |||
| return nil | |||
| } | |||
|
|
|||
| func pullModelManifest(ctx context.Context, mp ModelPath, regOpts *RegistryOptions) (*ManifestV2, error) { | |||
| func pullModelManifest(ctx context.Context, mp ModelPath, currentDigest string, regOpts *RegistryOptions) (*ManifestV2, error) { | |||
There was a problem hiding this comment.
This isn't how I imagined this working. It shouldn't be necessary to pass in the digest. If the manifest exists, it can calculate the digest and set If-None-Match. It will allow any pull request, not just an explicit upgrade, to reuse this logic
| if err != nil { | ||
| return fmt.Errorf("pull model manifest: %s", err) | ||
| } | ||
|
|
||
| if currentDigest != "" { | ||
| if manifest == nil { | ||
| // we already have the model |
There was a problem hiding this comment.
instead of silently returning, this should send something like model update to date
| resp, err := makeRequestWithRetry(ctx, http.MethodGet, requestURL, headers, nil, regOpts) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| defer resp.Body.Close() | ||
|
|
||
| // todo we can potentially read the manifest locally and return it here | ||
| if resp.StatusCode == http.StatusNotModified { | ||
| return nil, nil |
There was a problem hiding this comment.
returning nil, nil can be error prone. instead it should either return the current manifest, nil, or nil, os.ErrExist. either one can be explicitly handled.
returning the current manifest allows no changes in the upstream code which will print the progress as if it's pulling.
returning an error allows nicer handling for no updates
manifest, err := pullModelManifest()
if errors.Is(err, os.ErrExist) {
fn(api.ProgressResponse{Status: "model up-to-date"})
return nil
} else if err != nil {
return err
}
|
Are updates run synchronously or asynchronously? I've found that updating models in parallel is doable on a reasonably strong connection. |
|
@ThatOneCalculator each model is pulled synchronously, however, the "chunks" of that model are actually pulled asynchronously, which is how we get fast pull times. |
|
Ah. Would you consider adding multiple downloads at once? That might help speed up things even more. |
|
@ThatOneCalculator Given how we're already pulling things, I'm not sure that would help a lot. What speeds are you seeing when pulling right now? |
|
This would be an immensely useful feature but even better with functionality inside api/client.go maybe something like |
|
What is blocking this PR from getting merged? |
|
@KhimairaCrypto the etags implementation. It needs to be revamped. |
This change allows you to run
ollama pull --upgrade-allwhich will check each of your local models and upgrade any that are out of date. It uses Etags to check if there is a newer manifest, and then pulls that model if it has been updated.