-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Create a new export documents meilitool subcommand #4970
Conversation
5adea6e
to
e63b33c
Compare
Hey @dureuill 👋 I fixed the issue and added support for vectors. Can you review that again? 🙂 |
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.
I can accept the changes as-is, but I'm wondering if we should maybe factor the logic to export the documents between meilitool and the dumps:
this logic changed multiple times (most recently, to inject vectors) and now exists in two different locations. If the next change updates the index-scheduler but forgets about meilitool, then we will get possibly hard to track inconsistencies when using the meilitool export-documents command.
If factoring the logic is too complicated due to them being in different crates not using the same types, maybe a comment on the index-scheduler implementation to also implement the meilitool implementation would be welcome.
What do you think?
4e4d9e7
to
f222792
Compare
f222792
to
86fcad7
Compare
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.
thank you so much!
bors merge
Build succeeded:
|
This subcommand can be useful for extracting documents from an existing database.