-
Notifications
You must be signed in to change notification settings - Fork 5.1k
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
Add Falkordb vectorstore Integration #16047
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it look like more tests need to be added
|
||
_logger = logging.getLogger(__name__) | ||
|
||
def check_if_not_null(props: List[str], values: List[Any]) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this function only used once just inline it
|
||
def retrieve_existing_index(self) -> bool: | ||
index_information = self._driver.query( | ||
"SHOW INDEXES " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
replace with
CALL db.indexes()
def retrieve_existing_index(self) -> bool: | ||
index_information = self._driver.query( | ||
"SHOW INDEXES " | ||
"YIELD name, type, entityType, properties, options " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see output in https://docs.falkordb.com/cypher/procedures.html
filter_params = {} | ||
|
||
similarity_query = ( | ||
f"WITH n, vector.similarity.{self.distance_strategy}(" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see https://docs.falkordb.com/cypher/indexing.html#vector-indexing how to use vector indexing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it looks good now missing the docs and all the other files like in the other libs like pyproject.toml etc
@AviAvni please have a look at docs and if there's anything you think we need to change drop a comment, I'll fix it asap. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One small change
from llama_index.vector_stores.falkordb import FalkorDBVectorStore | ||
|
||
vector_store = FalkorDBVectorStore( | ||
url="bolt://localhost:7687", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change to falkor://
@logan-markewich can you please review/merge? |
yes, they were working fine for me, but if you need any help regarding anything, tag me and I will try my best to help you.
…On Sat, Oct 5, 2024 at 12:25 AM Logan ***@***.***> wrote:
@gkorland <https://github.com/gkorland> @AviAvni
<https://github.com/AviAvni> I fixed a handful of issues, but the unit
tests don't seem to work. Did you run them locally?
—
Reply to this email directly, view it on GitHub
<#16047 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/A5MFIX7JLYITNO3DTJLIXGTZZ3TTDAVCNFSM6AAAAABOJYEEZCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGOJUGQZTGOBYG4>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
@A5jadAli try running them, they don't actually work. Previously the mock db client wasn't being used (and the URL was also invalid) -- changed it slightly to use the mock client so that things are actually testable in CICD, but most tests fail (likely because things aren't mocked out entirely) |
Ok, I'll check it out.
…On Sat, Oct 5, 2024 at 9:54 PM Logan ***@***.***> wrote:
@A5jadAli <https://github.com/A5jadAli> try running them, they don't
actually work. Previously the mock db client wasn't being used (and the URL
was also invalid) -- changed it slightly to use the mock client so that
things are actually testable in CICD, but most tests fail (likely because
things aren't mocked out entirely)
—
Reply to this email directly, view it on GitHub
<#16047 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/A5MFIX2MHGZ5ZPU4J754DTDZ2AKVFAVCNFSM6AAAAABOJYEEZCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGOJVGEYTMMZXGU>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
This pull request introduces support for FalkorDB as a vectorstore option in LlamaIndex. The changes include: