-
Notifications
You must be signed in to change notification settings - Fork 79
models: return untagged model information on delete #100
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
pkg/inference/models/manager.go
Outdated
| w.Header().Set("Content-Type", "text/plain") | ||
| if _, err := w.Write([]byte(out)); err != nil { | ||
| http.Error(w, fmt.Sprintf("error writing response: %v", err), http.StatusInternalServerError) | ||
| } |
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 missed the review window on docker/model-distribution#104, but would it maybe make more sense to have DeleteModel return some sort of structure (e.g. []string) and then handle the formatting at the API and CLI layers? E.g. the corresponding DELETE /images response uses a JSON structure to format the results: https://docs.docker.com/reference/api/engine/version/v1.51/#tag/Image/operation/ImageDelete
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.
Done in docker/model-distribution#107, thanks!
3a960df to
efa6abe
Compare
08199e4 to
9474ef7
Compare
Bump model-distribution to docker/model-distribution@a633223 (current latest) to include docker/model-distribution@67a3bf2. Signed-off-by: Dorin Geman <[email protected]>
9474ef7 to
2bd09df
Compare
|
|
||
| w.Header().Set("Content-Type", "application/json") | ||
| if err := json.NewEncoder(w).Encode(resp); err != nil { | ||
| http.Error(w, fmt.Sprintf("error writing response: %v", err), http.StatusInternalServerError) |
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 not sure this'll do anything at this point if the encode fails mid-encode, but can't hurt I suppose.
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 guess it at least overwrites the previously set "Content-Type", "application/json" and send back an error.
* Added workflow to package a safetensors model * Build llama-converter image * No need to install git-lfs * Use linux/amd6 platform * No need for intermediate image stage llama-converter * remove duplicated FROM * Update README.md * add prerpare-matrix step as gguf workflow * fix example * llamacpp tag always came from the input * Update README.md
* Added workflow to package a safetensors model * Build llama-converter image * No need to install git-lfs * Use linux/amd6 platform * No need for intermediate image stage llama-converter * remove duplicated FROM * Update README.md * add prerpare-matrix step as gguf workflow * fix example * llamacpp tag always came from the input * Update README.md
use docker API client interfaces instead of concrete type
Return untagged model information on
DELETE <inference-prefix>/models/{name}requests.Bump model-distribution to docker/model-distribution@67a3bf2 (to also include docker/model-distribution@08f8ace) for the change of
m.distributionClient.DeleteModel.This is how the
docker model rmoutput will look like with these changes: