-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat: support for openai/anthropic compatibe apis #3824
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
|
this is awesome, thanks so much for doing this work! I had been dabbling myself in this and took a slightly different approach. I hope we can combine the two efforts here's my very much WIP: it rejigs the whole factory stuff and then adds custom providers that can be read from json. they have an engine, i.e. which provider executes it. that's kinda nice because then we don't have to have actually extra providers in there. I went with Ollama and OpenAI because I saw people ask for that, but we can easily add Anthropic of course |
Ahh!! This is an excellent idea! |
|
if you want to merge my PR into this and then make it all work, go for it! happy to close mine after that |
|
Ok, I'm going to give this a go to see if I can merge the two approaches. hold on! |
|
never mind, looks like you already did. checked out your branch and took it for a spin, this works pretty well! the only feedback I have here is that we could add a boolean to allow/disallow streaming (not all "compatible" backends support that) and we could consider moving some of the code that is being done in the config for the server (add custom provider) and the cli to the custom provider class so there's less repetition and we do less work in routes. can you merge master in so we can get a clean diff? looking forward to getting this in the hands of people! |
All resolved 🫡 |
DOsinga
left a comment
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.
Nice work:
- somehow the package-lock.json I think changed somehow
- have a look at my suggestion to do more work in the custom_provider.rs
- another thing to think about is that when you hit edit of a custom provider you get the default edit thing, instead of the configure custom provider
we can leave the last two for a follow up if you want though. would love to get this in for 1.3 which we should get ready for the coming week
ui/desktop/src/components/settings/providers/subcomponents/CardContainer.tsx
Show resolved
Hide resolved
DOsinga
left a comment
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.
can you fix the build things?
oops, resolved, should be all good now... |
|
can you have another look, they all seem to have failed. partially I think because there's changes to the package lock, but not to the actually installed packages. I still would love to get this in! also the commits are missing DCO which we sadly now require |
|
I'll make sure this gets merged. thanks |
|
thanks for doing this. I just merged this by cloning this and changing things, so I am going to close this, but hurray! |
🫡 ...#4099 |
|
let me know if you have any other good ideas you want to contribute! |
Fixes: #3777