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 Network.newNetwork() to create a Docker Network #1273

Closed

Conversation

siri-varma
Copy link
Contributor

@siri-varma siri-varma commented Mar 21, 2025

Description

From the logs it is not able to find "dapr-network"
The other nonflaky tests inside testcontainers just use Network.newNetwork(). So changed this to use the same pattern

Issue reference

We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.

Please reference the issue this PR will close: #1248

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

  • Code compiles correctly
  • Created/updated tests
  • Extended the documentation

Signed-off-by: sirivarma <[email protected]>
@siri-varma siri-varma requested review from a team as code owners March 21, 2025 04:56
@siri-varma
Copy link
Contributor Author

@artur-ciocanu , @salaboy , @cicoyle I just saw other non flaky tests use this pattern of creating a network. So changed this test to match that pattern as well. Let me know your thoughts

Signed-off-by: sirivarma <[email protected]>
@siri-varma siri-varma changed the title Change to get a normal network Use Network.newNetwork() to create a Docker Network Mar 21, 2025
@salaboy
Copy link
Contributor

salaboy commented Mar 21, 2025

@siri-varma we cannot use newNetwork() because we should be able to reuse the network if it already exists. This work locally, but for some reason it does fail in CI and it is becoming annoying, so I am investigating with @eddumelendez from testcontainers.

Copy link
Contributor

@salaboy salaboy left a comment

Choose a reason for hiding this comment

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

I dont think we can do this.

} else {
return defaultDaprNetwork;
}
return Network.newNetwork();
Copy link
Contributor

Choose a reason for hiding this comment

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

@siri-varma this would have been so much easier.. but we cannot do that if we want to run both applications locally to reuse the network.

@salaboy
Copy link
Contributor

salaboy commented Mar 21, 2025

@siri-varma can you please close this one?

@siri-varma
Copy link
Contributor Author

Synced up with @salaboy and looks like he is already working on it. Will close this PR.

@siri-varma siri-varma closed this Mar 22, 2025
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.

Fix Flaky CI Build Test Failure: ConsumerApp
2 participants