-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat: implement keyword and hybrid search for Weaviate provider #3264
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
feat: implement keyword and hybrid search for Weaviate provider #3264
Conversation
7de1961 to
9feecce
Compare
9feecce to
8f3bb59
Compare
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.
Some small nits but this looks mostly good. Thank you @ChristianZaccaria!
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.
Great work! Left small comments
|
My suggestions were addressed so lgtm. Very nice work. |
8493fad to
8daf980
Compare
|
lgtm |
8205a94 to
8949964
Compare
8949964 to
1053c1e
Compare
1053c1e to
d873408
Compare
d873408 to
c06c467
Compare
|
Weird, some unrelated tests failed, seem like a flake. Will push again to test. Edit: The tests passed now. |
897774d to
0e97b5e
Compare
183719d to
400b0aa
Compare
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.
final question otherwise LGTM
400b0aa to
82b41e3
Compare
82b41e3 to
fdedf17
Compare
fdedf17 to
920c2a3
Compare
|
Rebased, I think we are ready to merge. |
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.
lgtm, thanks for this @ChristianZaccaria!
Feedback addressed.
What does this PR do?
Closes #3010
Test Plan
Unit tests and integration tests should pass on this PR.