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

Add support for Cohere as model provider #42

Closed
wants to merge 6 commits into from
Closed

Add support for Cohere as model provider #42

wants to merge 6 commits into from

Conversation

daulet
Copy link

@daulet daulet commented Apr 27, 2024

Had to do some refactoring to plug in another provider, reviewing individual commits might be easier than the whole thing, but at high level:

  • Factor out OpenAI specific logic to OpenAIProvider type;
  • Factor out stdout writes out of http.Caller - that way other providers don't have to duplicate behavior embedded into http.Caller, which is currently used for making OpenAI calls;
  • Add CohereProvider that could be plugged in based on new provider field in configuration;

I don't know if you are open to supporting other providers at all, so publishing bigger PR than ideal to get initial buy in, can break it down into more consumable chunks if necessary.

If accepted it would probably require adding defaults per provider, such as default model, to make switching easier on user.

@kardolus
Copy link
Owner

@daulet wow, this is amazing. I am currently traveling but should have time to dive into this either on Sunday or Monday. Thanks!!

@daulet
Copy link
Author

daulet commented Apr 29, 2024

@kardolus the integration tests are failing due to this addition. To set provider we need to write to config file, so I added creating config directory on demand, but that fails the expectation in the tests. What would you recommend as the right fix here?

@kardolus
Copy link
Owner

kardolus commented May 3, 2024

@daulet thanks for your patience. Had a very busy week. Will look at the PR as a whole before diving into the integration tests. Will have time today.

@daulet daulet closed this by deleting the head repository May 24, 2024
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.

2 participants