Skip to content
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

update create handler to use model.Name #3737

Merged
merged 1 commit into from
Jun 5, 2024
Merged

update create handler to use model.Name #3737

merged 1 commit into from
Jun 5, 2024

Conversation

mxyng
Copy link
Contributor

@mxyng mxyng commented Apr 18, 2024

follow up to #3718

@mxyng mxyng force-pushed the mxyng/modelname-4 branch 2 times, most recently from 38bc91b to a94c401 Compare April 19, 2024 18:51
server/images.go Outdated Show resolved Hide resolved
@mxyng mxyng force-pushed the mxyng/modelname-3 branch 8 times, most recently from b2d8c1a to 075bb09 Compare May 8, 2024 18:11
@mxyng mxyng force-pushed the mxyng/modelname-4 branch 3 times, most recently from bb98227 to 898d0a8 Compare May 8, 2024 21:56
@mxyng mxyng force-pushed the mxyng/modelname-3 branch 2 times, most recently from 1cdbfc3 to 6a10185 Compare May 9, 2024 18:38
@mxyng mxyng force-pushed the mxyng/modelname-4 branch 2 times, most recently from 802b2ad to 69c0caf Compare May 9, 2024 18:56
Copy link
Contributor

@BruceMacD BruceMacD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a blocking bug, but I am unable to create a model in the default (library) namespace with this change. Maybe this should never have worked though:

./ollama create localhost:6000/nous-hermes-2-mistral -f /Users/bruce/models/nous-hermes-2-mistral/Modelfile
transferring model data 
Error: invalid model name

server/images.go Outdated Show resolved Hide resolved
server/manifest.go Outdated Show resolved Hide resolved
server/routes.go Outdated Show resolved Hide resolved
@mxyng mxyng force-pushed the mxyng/modelname-4 branch 2 times, most recently from 322c7dc to 5013cd0 Compare May 10, 2024 23:54
@mxyng
Copy link
Contributor Author

mxyng commented May 10, 2024

./ollama create localhost:6000/nous-hermes-2-mistral

you're setting host and model so this name is indeed not valid. to set host, you must also set namespace

@mxyng mxyng force-pushed the mxyng/modelname-3 branch 4 times, most recently from 8e957a7 to c5e892c Compare May 14, 2024 21:57
@mxyng mxyng force-pushed the mxyng/modelname-4 branch 3 times, most recently from 0e4d81e to d6d22ba Compare May 15, 2024 00:31
Base automatically changed from mxyng/modelname-3 to main May 29, 2024 19:02
Copy link
Contributor

@BruceMacD BruceMacD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good, but it seems as though the changes here have extended beyond model.Name a bit, unless I'm mis-reading the diff.

Looks like this may also need a rebase once the lint change goes in, so I'll review again then

if c.Name != "license" {
// replace
layers = slices.DeleteFunc(layers, func(layer *Layer) bool {
return layer.MediaType == mediatype
if layer.MediaType != mediatype {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's this logic change for? Doesnt seem related

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fixing a bug the new tests found. What's happening is on a template or system override, the layer being overwritten isn't being removed

@mxyng mxyng merged commit 22fcf8f into main Jun 5, 2024
12 checks passed
@mxyng mxyng deleted the mxyng/modelname-4 branch June 5, 2024 19:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants