testcontainers / testcontainers-dotnet

A library to support tests with throwaway instances of Docker containers for all compatible .NET Standard versions.
https://dotnet.testcontainers.org
MIT License
3.67k stars 254 forks source link

fix: Retry configuring Couchbase on HttpIOException #1064

Closed mgroves closed 7 months ago

mgroves commented 7 months ago

What does this PR do?

Added RetryHandler handler to the configuration, per discussion in issue #1062

Why is it important?

This is necessary because of the way Couchbase startup works very asynchronously. Various parts are not guaranteed to be ready in any particular order. This was causing 504 errors when making HTTP requests prematurely. A simple retry takes care of this.

Related issues

This should closes #1062

How to test this PR

Difficult to test, since the underlying issue is intermittent. However, the existing Couchbase tests passed with this updated code.

Follow-ups

My main concern is that the retry logic is pretty simple. If this issue crops up again, I would suggest lengthening the delay, increasing number of retries, and/or adding some kind of retry backoff period, and see if any of those help.

netlify[bot] commented 7 months ago

Deploy Preview for testcontainers-dotnet ready!

Name Link
Latest commit bee9f0a2ad4fc7087f61654dc42321684558d703
Latest deploy log https://app.netlify.com/sites/testcontainers-dotnet/deploys/6570a46366dd17000833e367
Deploy Preview https://deploy-preview-1064--testcontainers-dotnet.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

mgroves commented 7 months ago

The template said to add a label, but I don't seem to have the ability to do that. I just see "none yet" and no gear icon (yet).

HofmeisterAn commented 7 months ago

Thanks for creating the PR. I will look at it tomorrow. I would like to include a comment that explains why the retry delegation is necessary. But for today, I am done.

The template said to add a label, but I don't seem to have the ability to do that. I just see "none yet" and no gear icon (yet).

Oh, interesting. I was not aware of that. I will look into it. Thanks.

mgroves commented 7 months ago

And thank YOU for being so responsive, and providing excellent guidance :)