feat: add RedisKVStorage, MilvusVectorStorage, and NebulaGraphStorage #71
Conversation
|
Hi, MilvusStorage is already implemented, referring to this. We remove it from the package because the milvus-lite doesn't support window platform and we hope anyone can have a try of the default Also great work! |
|
Hi @utopia2077 , I think you should change your base branch to |
|
Hi, great PR. Can you consider remove the milvus vectorDB from the bulit-in? Because it doesn't support windows platform and will cause the windows users unable to install nano-graphrag. Besides that, the rest is really perfect! |
|
@wey-gu Would you might to review the part of NebulaGraph Storage in this PR? It will be a huge favor! |
| @@ -3,3 +3,6 @@ | |||
| from .vdb_hnswlib import HNSWVectorStorage | |||
| from .vdb_nanovectordb import NanoVectorDBStorage | |||
| from .kv_json import JsonKVStorage | |||
| from .kv_redis import RedisKVStorage | |||
| from .vdb_milvus import MilvusVectorStorage | |||
There was a problem hiding this comment.
Maybe remove this line, so the Milvus is not imported by default?
| neo4j | ||
| pymilvus |
There was a problem hiding this comment.
Remove the milvus dep, so those who want to use this component will handle the cross platform. Let's not introduce the install problems for the rest.
@gusye1234 Sure Will do! |
|
|
||
| class MyNebulaReader(NebulaReader): | ||
| """ | ||
| If the property has the name 'order', it will cause an error. |
There was a problem hiding this comment.
if we escape prop name with `, error still encountered?
| levels = defaultdict(set) | ||
|
|
||
|
|
||
| communities_result = self.client.execute_py( |
There was a problem hiding this comment.
this involves fullscan, we could create a index for COMMUNITY_VERTEX_TYPE tag to make this call more scalable :)
Ref: https://docs.nebula-graph.io/3.8.0/3.ngql-guide/14.native-index-statements/1.create-native-index/
If we are going do it in a programming way, a pitfall is that the index need to wait for two heartbeats of NebulaGraph cluster(default 20s) maximally to ensure it's synced across all metaDs
We could keep it as is or ask user to create them manually.
Note if we create index after the data was already there, we need to rebuild the index(see from the docs as well)
There was a problem hiding this comment.
I want to know why community_report_storage_cls still use JsonKVStorage ? Is there any problem
There was a problem hiding this comment.
It shouldn't actually our preparatory impl are full-graph-wise. Just to kept the original design of ms and nano graph-rag here.
TODO