feat: add Milvus vectorDB #1171
Conversation
zc277584121
requested review from
ashwinb,
yanxi0830,
hardikjshah,
dltn,
raghotham,
dineshyv,
vladimirivic,
sixianyi0721,
ehhuang and
terrytangyuan
as code owners
|
Hi @zc277584121! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at [email protected]. Thanks! |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
facebook-github-bot
added
the
CLA Signed
There was a problem hiding this comment.
@zc277584121 Thank you! @franciscojavierarceo given your prior experience on this, would you like to help review this PR?
|
yeah of course! |
| @@ -25,7 +25,7 @@ We are working on adding a few more APIs to complete the application lifecycle. | |||
|
|
|||
| The goal of Llama Stack is to build an ecosystem where users can easily swap out different implementations for the same API. Examples for these include: | |||
| - LLM inference providers (e.g., Fireworks, Together, AWS Bedrock, Groq, Cerebras, SambaNova, etc.), | |||
| - Vector databases (e.g., ChromaDB, Weaviate, Qdrant, FAISS, PGVector, etc.), | |||
There was a problem hiding this comment.
please also update the docs section as well under https://github.com/meta-llama/llama-stack/tree/main/docs/source/providers/vector_io/
See this PR as a reference #1195
|
|
||
|
|
||
| async def get_adapter_impl(config: MilvusVectorIOConfig, deps: Dict[Api, ProviderSpec]): | ||
| from .milvus import MilvusVectorIOAdapter |
There was a problem hiding this comment.
I think the inline and remote implementations are equivalent.
If that is, indeed, the case can you follow the pattern that's done by Chroma?
In short, they just import the remote implementation in the __init__.py and don't have an inline chromadb.py file.
|
|
||
| data = [] | ||
| for i, (chunk, embedding) in enumerate(zip(chunks, embeddings, strict=False)): | ||
| chunk_id = f"{chunk.metadata['document_id']}:chunk-{i}" |
There was a problem hiding this comment.
It's probably better to generate the chunk ID from the text and the document ID in case the order of insertion changes. See this function: https://github.com/meta-llama/llama-stack/blob/main/llama_stack/providers/inline/vector_io/sqlite_vec/sqlite_vec.py#L234
Maybe that can be moved to a utils folder.
| "chunk_content": chunk.model_dump(), | ||
| } | ||
| ) | ||
| self.client.insert( |
There was a problem hiding this comment.
Please have some error handling and log the collection name if insertion fails
| f"Chunk length {len(chunks)} does not match embedding length {len(embeddings)}" | ||
| ) | ||
| if not self.client.has_collection(self.collection_name): | ||
| self.client.create_collection(self.collection_name, dimension=len(embeddings[0]), auto_id=True) |
There was a problem hiding this comment.
I know bounded staleness is the the OOTB consistency configuration. It may be useful to make that explicit. Alternatively, I could see strong consistency being a useful default.
|
sure, I will fix them soon |
Signed-off-by: ChengZi <[email protected]>
Signed-off-by: ChengZi <[email protected]>
|
@franciscojavierarceo I have updated this PR to solve all the problems you mentioned, please check them again, thanks. |
|
Have updated the PR description and upload my end2end test logs into it, please check it. |
What does this PR do?
feat: add Milvus vectorDB
note: I use the MilvusClient to implement it instead of AsyncMilvusClient, because when I tested AsyncMilvusClient, it would raise issues about evenloop, which I think AsyncMilvusClient SDK is not robust enough to be compatible with llama_stack framework.
Test Plan
have passed the unit test and ene2end test
Here is my end2end test logs, including the client code, client log, server logs from inline and remote settings
test_end2end_logs.zip