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

Use Testcontainers Milvus module #783

Merged
merged 2 commits into from
Mar 5, 2024

Conversation

eddumelendez
Copy link
Contributor

@eddumelendez eddumelendez commented Mar 1, 2024

Testcontainers 1.19.6 provides a milvus module. It uses an embedded
etcd and local storage.

@sre-ci-robot sre-ci-robot requested review from yelusion2 and yhmo March 1, 2024 21:50
@sre-ci-robot
Copy link

Welcome @eddumelendez! It looks like this is your first PR to milvus-io/milvus-sdk-java 🎉

@eddumelendez
Copy link
Contributor Author

MilvusMultiClientDockerTest can also be moved because Testcontainers also provide a minio module and etcd can be used using GenericContainer.

Testcontainers 1.19.6 provides a milvus module. It uses an embedded
etcd and local storage.

Signed-off-by: Eddú Meléndez <eddu.melendez@gmail.com>
@yhmo
Copy link
Contributor

yhmo commented Mar 4, 2024

Hi @eddumelendez eddumelendez
Thank you for the pull request.
The CI is not passed. Could you double-check the cases in MilvusClientDockerTest and MilvusMultiClientDockerTest can pass in your local?

Failed while enforcing RequireUpperBoundDeps. The error(s) are [
Require upper bound dependencies error for org.slf4j:slf4j-api:1.7.30 paths to dependency are:
+-io.milvus:milvus-sdk-java:2.3.2
  +-org.slf4j:slf4j-api:1.7.30
and
+-io.milvus:milvus-sdk-java:2.3.2
  +-org.apache.logging.log4j:log4j-slf4j-impl:2.17.1
    +-org.slf4j:slf4j-api:1.7.25
and
+-io.milvus:milvus-sdk-java:2.3.2
  +-org.testcontainers:milvus:1.19.6
    +-org.testcontainers:testcontainers:1.19.6
      +-org.slf4j:slf4j-api:1.7.36
and
+-io.milvus:milvus-sdk-java:2.3.2
  +-org.testcontainers:milvus:1.19.6
    +-org.testcontainers:testcontainers:1.19.6
      +-com.github.docker-java:docker-java-api:3.3.5
        +-org.slf4j:slf4j-api:1.7.30
and
+-io.milvus:milvus-sdk-java:2.3.2
  +-org.testcontainers:milvus:1.19.6
    +-org.testcontainers:testcontainers:1.19.6
      +-com.github.docker-java:docker-java-transport-zerodep:3.3.5
        +-org.slf4j:slf4j-api:1.7.25
, 
Require upper bound dependencies error for org.jetbrains:annotations:13.0 paths to dependency are:
+-io.milvus:milvus-sdk-java:2.3.2
  +-org.jetbrains.kotlin:kotlin-stdlib:1.6.20
    +-org.jetbrains:annotations:13.0
and
+-io.milvus:milvus-sdk-java:2.3.2
  +-org.testcontainers:milvus:1.19.6
    +-org.testcontainers:testcontainers:1.19.6
      +-org.rnorth.duct-tape:duct-tape:1.0.8
        +-org.jetbrains:annotations:17.0.0
]

@eddumelendez
Copy link
Contributor Author

@yhmo PR updated. I have excluded the dependencies to use the ones provided by milvus itself. Tests are running fine locally.

Signed-off-by: Eddú Meléndez <eddu.melendez@gmail.com>
@yhmo
Copy link
Contributor

yhmo commented Mar 5, 2024

/lgtm
/approve

@sre-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: eddumelendez, yhmo

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@sre-ci-robot sre-ci-robot merged commit c972377 into milvus-io:master Mar 5, 2024
5 checks passed
@eddumelendez eddumelendez deleted the testcontainers branch March 5, 2024 15:07
@yhmo
Copy link
Contributor

yhmo commented Mar 6, 2024

With the testcontainers integrated, I ran the test case for the first time and encountered a timeout error that said the image testcontainers/ryuk:0.6.0 could not be pulled.
I manually download the image by docker pull testcontainers/ryuk:0.6.0 and the test case is succeed to pass.

Just note here for the people who may encounter this error in the future.

yhmo pushed a commit to yhmo/milvus-sdk-java that referenced this pull request Mar 6, 2024
* Use Testcontainers Milvus module

Testcontainers 1.19.6 provides a milvus module. It uses an embedded
etcd and local storage.

Signed-off-by: Eddú Meléndez <eddu.melendez@gmail.com>

* Polish pom

Signed-off-by: Eddú Meléndez <eddu.melendez@gmail.com>

---------

Signed-off-by: Eddú Meléndez <eddu.melendez@gmail.com>
yhmo pushed a commit to yhmo/milvus-sdk-java that referenced this pull request Mar 6, 2024
* Use Testcontainers Milvus module

Testcontainers 1.19.6 provides a milvus module. It uses an embedded
etcd and local storage.

Signed-off-by: yhmo <yihua.mo@zilliz.com>
sre-ci-robot pushed a commit that referenced this pull request Mar 6, 2024
* Use Testcontainers Milvus module

Testcontainers 1.19.6 provides a milvus module. It uses an embedded
etcd and local storage.

Signed-off-by: yhmo <yihua.mo@zilliz.com>
Co-authored-by: Eddú Meléndez Gonzales <eddu.melendez@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants