-
Notifications
You must be signed in to change notification settings - Fork 46
Enable creating an index "from_existing" #174
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
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.
installed_modules = unpack_redis_modules( | ||
convert_bytes(redis_client.module_list()) | ||
) | ||
validate_modules(installed_modules, [{"name": "search", "ver": 20810}]) |
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.
Is search the only one we need to validate? I'm looking at the remote possibility that somebody did their own module add in the configuration with mismatched versions, but that likely entail us to have a compatibility matrix with at least 3 columns, Redis version , Search module version, and JSON module version
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.
For the ability to read in from the FT.INFO output, it's not tied to any module besides Search. Not sure we can invest in supporting additional/ad-hoc modules (yet). Will likely require a bit of work to get there. But I do want to clean up the module and connection factory interfaces soon to make them a bit cleaner and more generic.
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.
Looks very good to me! 🚀
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.
LGTM!
2fc6733
to
80e9549
Compare
Since version 2.8.10 of the search module in Redis,
FT.INFO
now reports the contents of the index and associated fields and all low level attributes. Raw response from Redis:This enables the ability to "hydrate" a RedisVL
IndexSchema
class from this output. This makes state management for index information MUCH simpler. The caveat is that this capability is only available in newer versions of redis/search. So we have to hide it behind a "Feature flag" of sorts. This is directly needed in our integration clients like LangChain and LlamaIndex too.