Skip to content

Fix server readiness for 2.6 tests#123

Merged
roji merged 4 commits intomilvus-io:mainfrom
verdie-g:v2.6-test-readiness
Dec 12, 2025
Merged

Fix server readiness for 2.6 tests#123
roji merged 4 commits intomilvus-io:mainfrom
verdie-g:v2.6-test-readiness

Conversation

@verdie-g
Copy link
Copy Markdown
Contributor

Since #122, v2.6 tests have been failing with Grpc.Core.RpcException : Status(StatusCode="Unknown", Detail="service unavailable: internal: Milvus Proxy is not ready yet. please wait"). The testcontainer is checking /healthz but it seems like it's not enough. The proposed fix is to send some requests until the server is really ready.

{
DateTime before = DateTime.UtcNow;

await Task.Delay(200);
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.

Honestly, I'm a bit confused about this test.

@roji
Copy link
Copy Markdown
Collaborator

roji commented Nov 23, 2025

@verdie-g isn't this something that should be fixed in the milvus testcontainer itself? It's the testcontainer's responsibility to reliably wait until the service (Milvus in this case) is up and ready to handle requests. Do you want to submit a PR against https://github.com/testcontainers/testcontainers-dotnet/tree/develop/src/Testcontainers.Milvus?

@verdie-g
Copy link
Copy Markdown
Contributor Author

@verdie-g verdie-g marked this pull request as draft November 26, 2025 12:35
@verdie-g verdie-g marked this pull request as ready for review November 27, 2025 10:34
@verdie-g
Copy link
Copy Markdown
Contributor Author

@roji the testcontainer PR was merged. I'm copying its content here until it's released.

Copy link
Copy Markdown
Collaborator

@roji roji left a comment

Choose a reason for hiding this comment

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

Thanks for doing all this great work @verdie-g! I'll go ahead and merge for now, you can submit another PR once the new testcontainer is available to remove the unneeded stuff.

@roji
Copy link
Copy Markdown
Collaborator

roji commented Nov 27, 2025

@yhmo @xiaofan-luan any idea what the DCO status check is above? It's "waiting for status to be reported" and blocking merging.

@xiaofan-luan
Copy link
Copy Markdown
Contributor

https://milvus.io/blog/how-to-contribute-to-milvus-a-quick-start-for-developers.md

git commit -m "Commit of your change" -s

@verdie-g
could you double check you did "-s" to agree with DCO?

@verdie-g verdie-g force-pushed the v2.6-test-readiness branch from 51526d0 to 0cae3b7 Compare December 7, 2025 10:08
@verdie-g
Copy link
Copy Markdown
Contributor Author

verdie-g commented Dec 7, 2025

Thanks @xiaofan-luan, I've added the signoff but I can still see the "DCO Expected — Waiting for status to be reported". Any idea why?

@verdie-g verdie-g force-pushed the v2.6-test-readiness branch 5 times, most recently from 8244ead to aabbc71 Compare December 12, 2025 08:48
Signed-off-by: Grégoire Verdier <g.verdier@criteo.com>
Signed-off-by: Grégoire Verdier <g.verdier@criteo.com>
Signed-off-by: Grégoire Verdier <g.verdier@criteo.com>
@verdie-g verdie-g force-pushed the v2.6-test-readiness branch from aabbc71 to a01fc48 Compare December 12, 2025 08:52
@verdie-g verdie-g requested a review from roji December 12, 2025 09:10
@roji roji merged commit 55cfd41 into milvus-io:main Dec 12, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants