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

Fix: Couchbase containers intermittently hang on startup #2650

Merged
merged 3 commits into from
Jul 17, 2024

Conversation

couchbaseEd
Copy link
Contributor

@couchbaseEd couchbaseEd commented Jul 16, 2024

What does this PR do?

Checks the http response when creating the a primary index and retries if there is an error.

Why is it important?

When making the call to create the primary index the response isn't checked. This means that any errors returned in the response body are not caught and hence the isPrimaryIndexOnline polls until the timeout. In our case this occurs if we try to create the index to early before the bucket is ready.

Related issues

Run test script posted in original issue to try to reproduce. This is a timing issue so difficult to reproduce consistently.

@couchbaseEd couchbaseEd requested a review from a team as a code owner July 16, 2024 11:36
Copy link

netlify bot commented Jul 16, 2024

Deploy Preview for testcontainers-go ready!

Name Link
🔨 Latest commit bf985ff
🔍 Latest deploy log https://app.netlify.com/sites/testcontainers-go/deploys/6696a02c2fe970000888b1e4
😎 Deploy Preview https://deploy-preview-2650--testcontainers-go.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.

@couchbaseEd couchbaseEd force-pushed the fix/createPrimaryIndex branch from 838510f to a0c8e35 Compare July 16, 2024 15:08
@mdelapenya
Copy link
Member

@couchbaseEd I see a different changeset after the last push-force. Could it be possible that it's incorrect?

@couchbaseEd
Copy link
Contributor Author

ah I merged main and then squashed my commits. Just needed to update the branch

@mdelapenya mdelapenya self-assigned this Jul 17, 2024
@mdelapenya mdelapenya added the bug An issue with the library label Jul 17, 2024
Copy link
Member

@mdelapenya mdelapenya left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

Will merge when the CI passes

@mdelapenya mdelapenya merged commit 30d8c04 into testcontainers:main Jul 17, 2024
108 checks passed
mdelapenya added a commit that referenced this pull request Aug 5, 2024
* main:
  feat: add grafana-lgtm module (#2660)
  Added valkey module (#2639)
  fix: container.Endpoint and wait.FortHTTP to use lowest internal port (#2641)
  chore: test cleanups (#2657)
  docs: fix compilation of examples (#2656)
  feat: add custom container registry substitutor (#2647)
  fix: couchbase containers intermittently hang on startup (#2650)
  chore(deps): bump Ryuk to 0.8.1 (#2648)
  fix: retry on label error (#2644)
  perf: optimise docker authentication config lookup (#2646)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An issue with the library
Projects
None yet
2 participants