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: Milvus storage connector #1198

Merged
merged 2 commits into from May 20, 2024
Merged

Conversation

zc277584121
Copy link
Contributor

@zc277584121 zc277584121 commented Mar 29, 2024

Description
This PR adds support for Milvus - https://milvus.io/docs to be used as an archival storage connector.

Tests
Have tested successfully in local environment for tests/test_storage.py. and tests/test_load_archival.py

@zc277584121
Copy link
Contributor Author

@sarahwooders Can you help to review this PR, thanks

@sarahwooders
Copy link
Collaborator

Could you please make sure you ran poetry lock with Python 3.12 and poetry version 1.8.2?

@zc277584121
Copy link
Contributor Author

Could you please make sure you ran poetry lock with Python 3.12 and poetry version 1.8.2?

@sarahwooders Thank you for reminding me, and I think it's the correct setting now.

@sarahwooders
Copy link
Collaborator

Could you please make sure you ran poetry lock with Python 3.12 and poetry version 1.8.2?

@sarahwooders Thank you for reminding me, and I think it's the correct setting now.

Could you please double check you are using the right versions? It looks like the tests are still failing.

@zc277584121
Copy link
Contributor Author

Could you please make sure you ran poetry lock with Python 3.12 and poetry version 1.8.2?

@sarahwooders Thank you for reminding me, and I think it's the correct setting now.

Could you please double check you are using the right versions? It looks like the tests are still failing.

Hi, @sarahwooders
I'm sure that I'm using the right version.
image

I guess there may be other problems that caused this test to fail.

The tests failed due to hash poetry.lock file failing, I don't know what it means, I added milvus dependency in pyproject.toml, and updated poetry.lock file. Does this hash error indicate that I cannot modify the lock file? Or indicate other problems?
image

Can you please help to confirm or provide more information, thanks a lot.

@sarahwooders
Copy link
Collaborator

Could you please give me edit access to your branch, and i can play around with it? Not sure what is causing the error.

@zc277584121
Copy link
Contributor Author

@sarahwooders I have invited you as collaborator into my repo. Thanks for helping to check the problem

@zc277584121
Copy link
Contributor Author

Any progress on the debugging? @sarahwooders

@zc277584121
Copy link
Contributor Author

I added another commit "adapt to milvus lite" where deleted the milvus service starting from docker in githubworkflow file, instead, I use milvus lite to start milvus without any other new shell script. In this way, the hash file problem is fixed. However, there are still some problem with milvus lite, we will try to fix them as soon as possible.

Signed-off-by: ChengZi <chen.zhang@zilliz.com>
Signed-off-by: ChengZi <chen.zhang@zilliz.com>
@cpacker cpacker changed the base branch from main to milvus-staging May 20, 2024 06:15
Copy link
Collaborator

@sarahwooders sarahwooders left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm - merging into a staging branch so we can work on fixing some of the tests.

pyproject.toml Outdated Show resolved Hide resolved
@sarahwooders sarahwooders merged commit 4488410 into cpacker:milvus-staging May 20, 2024
5 of 8 checks passed
@zc277584121
Copy link
Contributor Author

@sarahwooders Thanks for the merging into milvus-staging. And here is my new pr: #1405. I updated the pymilvus and milvus-lite dependency, and made pymilvus optional. Now in the case of memgpt, the latest version of milvus-lite get the same capabilities as milvus docker service does. And I think it's currently good enough to be merged into main branch. Could you please help to review it?

cpacker pushed a commit that referenced this pull request May 26, 2024
Signed-off-by: ChengZi <chen.zhang@zilliz.com>
Co-authored-by: ChengZi <chen.zhang@zilliz.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants