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

feat: Qdrant support #81

Merged
merged 5 commits into from
Nov 4, 2024
Merged

feat: Qdrant support #81

merged 5 commits into from
Nov 4, 2024

Conversation

Anush008
Copy link
Contributor

@Anush008 Anush008 commented Oct 29, 2024

Description

Adds support for Qdrant - https://qdrant.tech/ to be used as vector store in Rig.

Resolves #2

Testing

I've QA tested and you can run the example with

export OPENAI_API_KEY=sk-********
docker run -p 6333:6333 -p 6334:6334 qdrant/qdrant
cargo run --release --example qdrant_vector_search

You can view the data at http://localhost:6333/dashboard

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules
  • I've reviewed the vector store API documentation and implemented the types of response accurately

Signed-off-by: Anush008 <[email protected]>
@0xMochan
Copy link
Contributor

0xMochan commented Oct 29, 2024

Hey, thanks for the PR! We appreciate your contributions! Before we take a closer look at your PR, can you fill in your PR description to add some more details? We have a PR template here which contains a format that should help in organizing more info about your PR. I would also include some specific points about implementation, especially since the team might not be as familiar with this vector store and some of the quirks that might be different than other ones (like I'm unsure why QueryPointsBuilder and UUIDs need to be exposed in public instead of being handled internally).

Also, this PR touches some rig-core and rig-mongodb code (in the examples) which seems like a mistake. Was this intended?

Lastly, for testing, we don't have our integration testing fully figured out (as per #68). When we do, we will come back to this and add testing. I would include a description on how you verified and manually tested this integration with clear instructions for us to replicate so that we can better test this PR and perhaps in the future, leverage it for some automated integration testing.

@Anush008
Copy link
Contributor Author

Also, this PR touches some rig-core and rig-mongodb code (in the examples) which seems like a mistake. Was this intended?

That was from

cargo clippy --fix

@Anush008
Copy link
Contributor Author

Lastly, for testing, we don't have our integration testing fully figured out (as per #68). When we do, we will come back to this and add testing. I would include a description on how you verified and manually tested this integration with clear instructions for us to replicate so that we can better test this PR and perhaps in the future, leverage it for some automated integration testing.

Added to the PR description

@Anush008
Copy link
Contributor Author

like I'm unsure why QueryPointsBuilder and UUIDs need to be exposed in public instead of being handled internally).

Could you elaborate in the diff?

Copy link
Contributor

@0xMochan 0xMochan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just a quick review, I (or someone else) will do a proper review later!

rig-qdrant/examples/qdrant_vector_search.rs Show resolved Hide resolved
rig-qdrant/src/lib.rs Outdated Show resolved Hide resolved
rig-qdrant/examples/qdrant_vector_search.rs Show resolved Hide resolved
Copy link
Contributor

@cvauclair cvauclair left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nicely done! Thank you for your contribution 😁

rig-qdrant/examples/qdrant_vector_search.rs Show resolved Hide resolved
rig-qdrant/examples/qdrant_vector_search.rs Show resolved Hide resolved
@cvauclair
Copy link
Contributor

Actually @Anush008 I did not realize that CI had not been run yet, if you could address the failing checks we can merge after that!

@Anush008
Copy link
Contributor Author

Anush008 commented Nov 4, 2024

Hey @cvauclair. Could you please approve the CI?

@cvauclair cvauclair merged commit bdfe1ba into 0xPlaygrounds:main Nov 4, 2024
4 checks passed
@Anush008 Anush008 deleted the qdrant branch November 4, 2024 17:28
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.

feat(datastore): add Qdrant data store
3 participants