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

gh-120 builder for generative openai module #124

Merged

Conversation

shreyanshshah27
Copy link
Contributor

No description provided.

@shreyanshshah27 shreyanshshah27 force-pushed the gh-120-generative-openai-module-builder branch from 38379f4 to c4e87a5 Compare February 19, 2023 16:13
@shreyanshshah27
Copy link
Contributor Author

Hey @dirkkul

I've tried to implement the builder for the generative-openai module. I've also added a few test cases to verify the feature. Integration test is failing right now because the OPENAI_APIKEY hasn't been added to the secrets yet. It would be great if you can add this key to the secrets and re-run the tests.

Also let me know your thoughts on this builder implementation. If this looks good then I'll add the remaining few integration tests for this builder.

Also in the docs it is mentioned that this would work with Get{} only, but as far as I understand this can work with MultiClassBuilder also.

@dirkkul
Copy link
Collaborator

dirkkul commented Feb 20, 2023

Hi @shreyanshshah27

thanks a lot for this! I didn't have time to review this, will try tomorrow.

I don't have an OpenAI key that I can put into the CI yet, will try to organize one. But could you redo the tests, so they run without that key and just skip tests that need one? I did something similar for the authentication tests here.

PS if you install pre-commit gofumpt is run on every commit and your files are reformated automatically.

@shreyanshshah27
Copy link
Contributor Author

@dirkkul No worries!! Please take your time. As you suggested, I've fixed the generative-module integration tests. But I'm facing some issue while installing gmfumpt, will try to get it resolved before next commit xD.

@dirkkul
Copy link
Collaborator

dirkkul commented Feb 21, 2023

great thank you!

I added the OPENAI_APIKEY env variable. The tests won't run for you as secrets aren't shared with external contributors, but I'll test it locally. And after merging it will run :)

If you need any help with gofumpt, you can also ping me on slack

@weaviate-git-bot
Copy link

Great to see you again! Thanks for the contribution.

beep boop - the SeMI bot 👋🤖

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

test/testsuit/generics.go Show resolved Hide resolved
test/testsuit/generics.go Outdated Show resolved Hide resolved
weaviate/graphql/generativesearch.go Show resolved Hide resolved
weaviate/graphql/generativesearch.go Outdated Show resolved Hide resolved
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