refactor: ListModels Filtering Upgrade #2773
Conversation
Signed-off-by: Dave Lee <[email protected]>
Signed-off-by: Dave Lee <[email protected]>
…e files Signed-off-by: Dave Lee <[email protected]>
✅ Deploy Preview for localai ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Signed-off-by: Dave Lee <[email protected]>
Signed-off-by: Dave Lee <[email protected]>
Signed-off-by: Dave Lee <[email protected]>
Signed-off-by: Dave Lee <[email protected]>
Signed-off-by: Dave Lee <[email protected]>
Signed-off-by: Dave Lee <[email protected]>
Signed-off-by: Dave Lee <[email protected]>
dave-gray101
changed the title
dave-gray101
changed the title
dave-gray101
changed the title
dave-gray101
changed the title
dave-gray101
changed the title
Signed-off-by: Dave Lee <[email protected]>
…1/LocalAI into gw-list-model-filter-upgrade
Signed-off-by: Dave Lee <[email protected]>
Signed-off-by: Dave Lee <[email protected]>
Signed-off-by: Dave Lee <[email protected]>
|
@mudler mind reviewing this when you get a chance? It's been sitting a while, so I did just merge master to shake out the dust. |
Signed-off-by: Dave Lee <[email protected]>
Signed-off-by: Dave Lee <[email protected]>
core/config/backend_config.go
Outdated
| // | ||
| // In its current state, this function should ideally check for properties of the config like templates, rather than the direct backend name checks for the lower half. | ||
| // This avoids the maintenance burden of updating this list for each new backend - but unfortunately, that's the best option for some services currently. | ||
| func (c *BackendConfig) HasUsecases(u BackendConfigUsecases) bool { |
There was a problem hiding this comment.
My two cents on this - I would expect a "category" field on the backend config at this point. Hooking into the backend name can be brittle, mainly because a user might expand backends with --external-backends. Maybe we can mix (detection) and explicit configuration via backend config?
There was a problem hiding this comment.
As I mentioned in that comment, I'm significantly concerned with the maintenance burden of our yaml config backlog [to either initially create it, or update if new backends are created].
I see a few paths forward:
- We could ignore heuristics if category is set - this is the simplest and sanest option, but it's got possible future tech debt in the form of "we add a new usecase" or any other major breaking change. Existing yaml configs would run through the heuristic, but new ones could be created to bypass it.
- We could just use both. If a model has a category field, we also run it through heuristics and use the union. At worst, we show a model in a place it's not super useful, which is what we currently do everywhere. This is my current plan
- PR updated to demonstrate this
- A more dramatic solution would be to adapt this heuristic function to a "one off" script that we could run against existing gallery files. If we added the category field to everything that exists today, we could mandate that it's a requirement going forward, and eliminate the need for runtime scanning. This does run the risk of "new usecase tech debt", but it seems saner to require an "update the gallery" script?
Signed-off-by: Dave Lee <[email protected]>
Signed-off-by: Dave Lee <[email protected]>
Signed-off-by: Dave Lee <[email protected]>
Signed-off-by: Dave Lee <[email protected]>
Signed-off-by: Dave Lee <[email protected]>
…1/LocalAI into gw-list-model-filter-upgrade
Signed-off-by: Dave Lee <[email protected]>
Signed-off-by: Dave Lee <[email protected]>
Signed-off-by: Dave Lee <[email protected]>
| } | ||
| } | ||
| if (u & FLAG_TTS) == FLAG_TTS { | ||
| ttsBackends := []string{"piper", "transformers-musicgen", "parler-tts"} |
There was a problem hiding this comment.
another optimization down the line is to extract the backend list here at the top of the file in a map or either a list for easing out maintenance
There was a problem hiding this comment.
I like that idea. I plan to do some more refactoring around the services, and I think the best place to locate that list will be over there. Thanks!
ttsService.GetKnownBackends() or something similar - not ruling out putting it above here, just seems a bit strange to maintain an ongoing list of backends in the config subdir :)
|
this coqui test started to be flaky lately, ugh |
| @@ -21,12 +21,12 @@ func ModelFromContext(ctx *fiber.Ctx, cl *config.BackendConfigLoader, loader *mo | |||
| } | |||
|
|
|||
| // Set model from bearer token, if available | |||
| bearer := strings.TrimLeft(ctx.Get("authorization"), "Bearer ") | |||
| bearer := strings.TrimLeft(ctx.Get("authorization"), "Bear ") // Reduced duplicate characters of Bearer | |||
There was a problem hiding this comment.
Interesting enough it is not. The static analysis tooling we've been experimenting with pointed this out to me and I verified it in the golang source: https://github.com/golang/go/blob/master/src/strings/strings.go
TrimLeft (and its many friends) iterate through the string a single time, checking every character. If that character is contained in the "cutset" string even once, it will be removed. No need for duplicated characters - they will be scanned M*N every time its called, without benefit. This example is B e a r er
I have been baby sitting that one for a few weeks now - it's unfortunately not related to PR content and is something we'll need to address separately. |
Description
This PR introduces just the ListModels and
configpackage changes from my earlier middleware PR. In order to manage size and conflicts, this PR does not use Usecase-based filtering yet - simply migrates existing name based filtering to the new style and sets up the initial test to prove usecases are working.In the upcoming [new] middleware PR, default models will take this into account and we can drive better dropdowns on the UI as well.