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: NebulaGraph as persistant layer #9

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

wey-gu
Copy link

@wey-gu wey-gu commented Aug 19, 2024

partially-implement: #2

@wey-gu
Copy link
Author

wey-gu commented Aug 19, 2024

I realize that implementing NebulaGraph as the BaseKVStore is also necessary to fully leverage its graph capabilities.

This is still a working in progress and requires more effort.

@gusye1234
Copy link
Owner

Really COOL PR! Just a quick reminder:
I noticed NebulaGraphStorage has many dataclass field with default value.
However the Instantiation of NebulaGraphStorage is controlled by nano-graphrag, not the user. So those fields will always remain the default values. I'm not sure that's what you want.

If not, all the parameters of GraphRAG class will be passed into BaseStorage as self.global_config, maybe you can read self.global_config['addon_params'] to make sure user can overwrite some parameters of NebulaGraphStorage

@wey-gu
Copy link
Author

wey-gu commented Aug 21, 2024

Really COOL PR! Just a quick reminder: I noticed NebulaGraphStorage has many dataclass field with default value. However the Instantiation of NebulaGraphStorage is controlled by nano-graphrag, not the user. So those fields will always remain the default values. I'm not sure that's what you want.

If not, all the parameters of GraphRAG class will be passed into BaseStorage as self.global_config, maybe you can read self.global_config['addon_params'] to make sure user can overwrite some parameters of NebulaGraphStorage

Thanks! Got it, some of them indeed are configurable and I'll use addon_params!

- [x] as BaseGraphStorage
- [ ] as BaseKVStorage
- [x] as BaseGraphStorage
- [-] as BaseKVStorage # <--- this commit is working on this
@wey-gu
Copy link
Author

wey-gu commented Aug 21, 2024

update aug-21 7ff213a:

  • WIP adding knowledge storage with NebulaGraph(some of KVStorage namespaces will be graph native in NebulaGraph), some are not(llm cache)
  • rebased the main to resolve conflict

@gusye1234
Copy link
Owner

update aug-21 7ff213a:

  • WIP adding knowledge storage with NebulaGraph(some of KVStorage namespaces will be graph native in NebulaGraph), some are not(llm cache)
  • rebased the main to resolve conflict

Hi, wey. Great work and design. StorageFactory is great design for fine-grained control.
But maybe making StorageFactory an optional schema for initialization is a better idea?
I think it kinda high-level design and the main path of nano-graphrag should be simple and easy.

Users should firstly understand the BaseStorage concepts(kv, gragh, vector) and play with it, then the detailed components (full_docs, text chunks...) configures.

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.

2 participants