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 KubeAI model provider integration #2553

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

samos123
Copy link
Contributor

@samos123 samos123 commented Sep 6, 2024

What's being changed:

Add KubeAI as a model provider to https://weaviate.io/developers/weaviate/model-providers

Type of change:

  • Documentation updates (non-breaking change to fix/update documentation)

How Has This Been Tested?

  • GitHub action – automated build completed without errors
  • Local build - the site works as expected when running yarn start

note, you can run yarn verify-links to test site links locally

I tested this using steps documented here: https://github.com/substratusai/kubeai/blob/main/docs/tutorials/weaviate.md

@samos123 samos123 force-pushed the kubeai-integration branch 8 times, most recently from d2a6aa2 to 6a38738 Compare September 6, 2024 19:39
@weaviate-git-bot
Copy link

To avoid any confusion in the future about your contribution to Weaviate, we work with a Contributor License Agreement. If you agree, you can simply add a comment to this PR that you agree with the CLA so that we can merge.

beep boop - the Weaviate bot 👋🤖

PS:
Are you already a member of the Weaviate Slack channel?

@samos123
Copy link
Contributor Author

samos123 commented Sep 9, 2024

Agree

@samos123 samos123 marked this pull request as ready for review September 9, 2024 21:34
@samos123
Copy link
Contributor Author

Could I get a review please? @databyjp

Please just let me know directly if you prefer not to merge this. I can close this PR, np at all.

@databyjp
Copy link
Contributor

Could I get a review please? @databyjp

Please just let me know directly if you prefer not to merge this. I can close this PR, np at all.

Hey @samos123, thank you for this and sorry about the delay. We do want to move forward with this in some way.

We have been just internally discussing how to best do it. It may not be ideal to ask users to pass on dummy API keys and so on, so we are internally discussing what to do from our module perspective, perhaps to create one for "openai-compatible" APIs.

@samos123
Copy link
Contributor Author

I think the UX of re-using existing modules would be better. Not having to enable new modules and out of the box compatibility with all existing Weaviate versions.

Note that right now KubeAI ignores the API key field, but this may change in the future. The plan is to allow users to configure an API key. So I wouldn't use configuring "dummy API keys" as a reason for having a separate module. I'm fine with whatever the Weaviate team thinks is the best path forward. Just sharing my pov in case it's helpful.

Happy to join any discussions as well.

@databyjp
Copy link
Contributor

Thanks Sam. Yeah, I do see some of those points.

From my perspective, we are looking at framing things a little more user-centrically. So, we've added:

  • ENABLE_API_BASED_MODULES to enable all API-based modules under the hood, and
  • Reframed our docs to be around each provider, as you have seen obviously.

I personally worry about coupling many of these keys and features around one module and client methods, which would make them difficult to untangle in case of changes.

As an example, it would be easier to have the python docstring clearly point to a generic "OpenAI-compatible" API docs that people can build around, vs an actual OpenAI module. We have actually separated the azure_openai factory method for this reason, as it was confusing for our users.

I do appreciate that it's probably not a slam dunk easy decision one way or another for now, but this is the way we are looking at it currently.

@databyjp
Copy link
Contributor

databyjp commented Sep 16, 2024

Hey @samos123 - what do you think about this:

We could go ahead with some version of this PR for now, with the proviso that the actual code will be swapped out at some time with an openai_compatible type module or factory method?

Edit: This way we are not holding you back while we figure out what to do at our end 😄

@samos123
Copy link
Contributor Author

samos123 commented Sep 16, 2024

Yeah that works for me. I would be happy to collaborate and test the new module as well.

Feel free to make any modifications to this PR directly as well.

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.

3 participants