You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I have looked for existing issues (including closed) about this
Feature Request
Some of the vector stores (like the Qdrant one) do not implement Clone. This can cause issues when writing web applications and attempting to include them in shared application state without knowledge of how to use std::sync::Arc. Please see the Discord conversation snippet for more context:
[19:22]josh from shuttle: it looks like there is no function to add embeddings to a given database through the EmbeddingModel trait - is this intended? the Qdrant client doesn't implement Clone, so users have to create a new qdrant connection to be able to upsert records/embeddings manually
(note that EmbeddingModel already requires Clone, so if we just make the internal client Arc where necessary, it should be ok to derive Clone from - which will also additionally make it much easier to use with web applications. I can kind of get around it by just cloning an arc then using into_inner but it's still annoying to do so)
[20:16]stopher.pg | UTC-5: Yeah this is intended behavior! We used to have a trait to insert documents into vector DBs, but this introduced constraints on the data model used in the vector DB (e.g.: you could not create a VectorStoreIndex on an existing DB if it didnt match the format the client uses to insert data).
Right now the responsibility of inserting data into the store is outside the scope of Rig (might be a fresh DB populated within the same process, or an exsiting DB populated by another process entirely) and falls on the user to implement.
That being said, we can definitely change QdrantVectorStore to use Arc<qdrant_client::Qdrant> instead if that makes it easier! Could you create an issue and/or PR for this so it doesnt get lost? Thanks for spotting this!
[20:16]josh from shuttle: Yeah no worries - ok if it's intended then I think just changing it to use Arc internally and adding the new method (to get T) would probably be the answer here
Motivation
Improves DX
Proposal
Make the internal client Arc<T> if it doesn't implement Clone and add a method to get the internal client. This will additionally unlock extra use cases for users who want access to lower-level functionality.
Alternatives
I am not sure what the alternative is here, if any.
The text was updated successfully, but these errors were encountered:
Feature Request
Some of the vector stores (like the Qdrant one) do not implement Clone. This can cause issues when writing web applications and attempting to include them in shared application state without knowledge of how to use
std::sync::Arc
. Please see the Discord conversation snippet for more context:[19:22]josh from shuttle: it looks like there is no function to add embeddings to a given database through the EmbeddingModel trait - is this intended? the Qdrant client doesn't implement Clone, so users have to create a new qdrant connection to be able to upsert records/embeddings manually
(note that EmbeddingModel already requires Clone, so if we just make the internal client Arc where necessary, it should be ok to derive Clone from - which will also additionally make it much easier to use with web applications. I can kind of get around it by just cloning an arc then using into_inner but it's still annoying to do so)
[20:16]stopher.pg | UTC-5: Yeah this is intended behavior! We used to have a trait to insert documents into vector DBs, but this introduced constraints on the data model used in the vector DB (e.g.: you could not create a VectorStoreIndex on an existing DB if it didnt match the format the client uses to insert data).
Right now the responsibility of inserting data into the store is outside the scope of Rig (might be a fresh DB populated within the same process, or an exsiting DB populated by another process entirely) and falls on the user to implement.
That being said, we can definitely change QdrantVectorStore to use Arc<qdrant_client::Qdrant> instead if that makes it easier! Could you create an issue and/or PR for this so it doesnt get lost? Thanks for spotting this!
[20:16]josh from shuttle: Yeah no worries - ok if it's intended then I think just changing it to use Arc internally and adding the new method (to get T) would probably be the answer here
Motivation
Improves DX
Proposal
Make the internal client
Arc<T>
if it doesn't implement Clone and add a method to get the internal client. This will additionally unlock extra use cases for users who want access to lower-level functionality.Alternatives
I am not sure what the alternative is here, if any.
The text was updated successfully, but these errors were encountered: