Skip to content
This repository was archived by the owner on Sep 9, 2026. It is now read-only.

feat: support milvus - #1681

Merged
JoanFM merged 16 commits into
mainfrom
feat-support-milvus
Jul 19, 2023
Merged

feat: support milvus#1681
JoanFM merged 16 commits into
mainfrom
feat-support-milvus

Conversation

@jupyterjazz

@jupyterjazz jupyterjazz commented Jul 2, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

Taking over from here #1666

  • Find (vector search)
  • Filter
  • Get/Del
  • Hybrid Search (find+filter)
  • Subindex
  • Tests

Functionality-wise everything's ready, only thing left is to make the execution faster (milvus job takes 13 mins on CI)

Docs will be done in a separate PR

Note:
I had to bump protobuf and tensorflow versions

jupyterjazz and others added 2 commits July 2, 2023 23:16
@jupyterjazz
chore: first commit
b0c675b 
Signed-off-by: jupyterjazz <saba.sturua@jina.ai>
@maxwelljin @jupyterjazz
feat: support milvus (#1666)
0e24ce9 
Signed-off-by: maxwelljin2 <gejin@berkeley.edu>
Co-authored-by: Saba Sturua <45267439+jupyterjazz@users.noreply.github.com>
@github-actions

github-actions Bot commented Jul 2, 2023

Copy link
Copy Markdown

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

@jupyterjazz jupyterjazz linked an issue Jul 2, 2023 that may be closed by this pull request
Feature: please add milvus as storage backend to docarray v2 #1549
Closed
@jupyterjazz
refactor: some changes
d1edd96 
Signed-off-by: jupyterjazz <saba.sturua@jina.ai>
@jupyterjazz
fix: merge conflicts
2113cdd 
Signed-off-by: jupyterjazz <saba.sturua@jina.ai>
@github-actions

github-actions Bot commented Jul 13, 2023

Copy link
Copy Markdown

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

@codecov

codecov Bot commented Jul 13, 2023 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Patch coverage: 90.70% and project coverage change: +0.17 🎉

Comparison is base (b306c80) 85.51% compared to head (059a8f4) 85.68%.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #1681      +/-   ##
==========================================
+ Coverage   85.51%   85.68%   +0.17%     
==========================================
  Files         132      133       +1     
  Lines        8303     8566     +263     
==========================================
+ Hits         7100     7340     +240     
- Misses       1203     1226      +23
Flag Coverage Δ
docarray 85.68% <90.70%> (+0.17%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
docarray/index/backends/redis.py 88.93% <ø> (+0.23%) ⬆️
docarray/utils/_internal/misc.py 65.57% <ø> (ø)
docarray/index/__init__.py 82.50% <80.00%> (+4.72%) ⬆️
docarray/index/backends/milvus.py 90.76% <90.76%> (ø)
docarray/index/backends/hnswlib.py 95.33% <100.00%> (+0.05%) ⬆️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@jupyterjazz
chore: extra name
d8fdb49 
Signed-off-by: jupyterjazz <saba.sturua@jina.ai>
@github-actions

github-actions Bot commented Jul 13, 2023

Copy link
Copy Markdown

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

@jupyterjazz
chore: refactor extra
93f2dc0 
Signed-off-by: jupyterjazz <saba.sturua@jina.ai>
@github-actions

github-actions Bot commented Jul 13, 2023

Copy link
Copy Markdown

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

@jupyterjazz
chore: install milvus
06d0602 
Signed-off-by: jupyterjazz <saba.sturua@jina.ai>
@github-actions

github-actions Bot commented Jul 13, 2023

Copy link
Copy Markdown

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

@jupyterjazz
fix: installing pymilvus
30d133b 
Signed-off-by: jupyterjazz <saba.sturua@jina.ai>
@github-actions

github-actions Bot commented Jul 13, 2023

Copy link
Copy Markdown

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

@jupyterjazz
chore: bump tensorflow
9ddb0ff 
Signed-off-by: jupyterjazz <saba.sturua@jina.ai>
@github-actions

github-actions Bot commented Jul 13, 2023

Copy link
Copy Markdown

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

@jupyterjazz
refactor: collection loading
eff5ab1 
Signed-off-by: jupyterjazz <saba.sturua@jina.ai>
@github-actions

github-actions Bot commented Jul 14, 2023

Copy link
Copy Markdown

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

@jupyterjazz
feat: subindex
da8187e 
Signed-off-by: jupyterjazz <saba.sturua@jina.ai>
@github-actions

github-actions Bot commented Jul 14, 2023

Copy link
Copy Markdown

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

@jupyterjazz
feat: configurations
cf8aa92 
Signed-off-by: jupyterjazz <saba.sturua@jina.ai>
@github-actions

github-actions Bot commented Jul 14, 2023

Copy link
Copy Markdown

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

@jupyterjazz
feat: query builder
87d487d 
Signed-off-by: jupyterjazz <saba.sturua@jina.ai>
@github-actions

github-actions Bot commented Jul 14, 2023

Copy link
Copy Markdown

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

@jupyterjazz
refactor: minor adjustments
6dce316 
Signed-off-by: jupyterjazz <saba.sturua@jina.ai>
@github-actions

github-actions Bot commented Jul 14, 2023

Copy link
Copy Markdown

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

@jupyterjazz
jupyterjazz marked this pull request as ready for review July 14, 2023 19:00
@jupyterjazz
refactor: adjust to recent changes
92d05c6 
Signed-off-by: jupyterjazz <saba.sturua@jina.ai>
@github-actions

github-actions Bot commented Jul 17, 2023

Copy link
Copy Markdown

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

samsja
samsja reviewed Jul 18, 2023
View reviewed changes
Comment thread docarray/index/backends/milvus.py
Comment on lines +183 to +192
type_map = {
int: DataType.INT64,
float: DataType.FLOAT,
str: DataType.VARCHAR,
bytes: DataType.VARCHAR,
np.ndarray: DataType.FLOAT_VECTOR,
list: DataType.FLOAT_VECTOR,
AnyTensor: DataType.FLOAT_VECTOR,
AbstractTensor: DataType.FLOAT_VECTOR,
}

@samsja samsja Jul 18, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We can move this dict away from the function imo

@jupyterjazz jupyterjazz Jul 18, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

hmm but all other backends have it inside

samsja
samsja reviewed Jul 18, 2023
View reviewed changes

@samsja samsja left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

added one minor comments

@jupyterjazz
chore: merge recent changes
059a8f4 
Signed-off-by: jupyterjazz <saba.sturua@jina.ai>
@github-actions

github-actions Bot commented Jul 18, 2023

Copy link
Copy Markdown

This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR might be rejected due to its size.

@github-actions

github-actions Bot commented Jul 18, 2023

Copy link
Copy Markdown

📝 Docs are deployed on https://ft-feat-support-milvus--jina-docs.netlify.app 🎉

samsja
samsja reviewed Jul 19, 2023
View reviewed changes
Comment thread .github/workflows/ci.yml
python -m pip install poetry
poetry install --all-extras
poetry run pip install protobuf==3.19.0 # we check that we support 3.19
poetry run pip install protobuf==3.20.0 # we check that we support 3.19

@samsja samsja Jul 19, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
poetry run pip install protobuf==3.20.0 # we check that we support 3.19
poetry run pip install protobuf==3.20.0 # we check that we support 3.20

@JoanFM
JoanFM merged commit 007f113 into main Jul 19, 2023
@JoanFM
JoanFM deleted the feat-support-milvus branch July 19, 2023 11:22
@jupyterjazz jupyterjazz mentioned this pull request Aug 1, 2023
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@JohannesMessner JohannesMessner Awaiting requested review from JohannesMessner
2 more reviewers
@JoanFM JoanFM JoanFM approved these changes
@samsja samsja samsja approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area/cicd area/core area/housekeeping area/setup area/testing size/xl

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Feature: please add milvus as storage backend to docarray v2

4 participants

@jupyterjazz @JoanFM @samsja @maxwelljin