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

feat(ryuk): allow to configure ryuk timeouts using env variables #2541

Conversation

fcx-mrogez
Copy link
Contributor

What does this PR do?

  • Add the possibility to use environment variables to configure ryuk timeouts (reconnection and connection).
  • Update unit tests
  • Update documentation
  • Fix up a minor typo in tests

Why is it important?

Default timeout are not suitable in all situations :

  • tests taking longer than 10s gets killed before completing (and may make also the following test fail from what I've observed)
  • if image building / pulling takes more than 1m to complete, ryuk will timeout and make the test fail.

Configuring these timeouts is currently done via property file which must be located in user home.
This is not very convenient in CI context (need to create proper property file in runner home).
For some common configuration settings, such as TESTCONTAINERS_RYUK_{DISABLE,VERBOSE} it is possible to use env variable which is much more convenient. So we extend this logic to ryuk timeouts.

Related issues

Couldn't find any.

@fcx-mrogez fcx-mrogez requested a review from a team as a code owner May 17, 2024 09:17
Copy link

netlify bot commented May 17, 2024

Deploy Preview for testcontainers-go ready!

Name Link
🔨 Latest commit 8df345a
🔍 Latest deploy log https://app.netlify.com/sites/testcontainers-go/deploys/6647209265311e000834ed7b
😎 Deploy Preview https://deploy-preview-2541--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.

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!

@mdelapenya mdelapenya self-assigned this May 20, 2024
@mdelapenya mdelapenya added the feature New functionality or new behaviors on the existing one label May 20, 2024
@mdelapenya
Copy link
Member

Thanks for improving the behaviour in the library! Much appreciated 👏

@mdelapenya mdelapenya merged commit 741d438 into testcontainers:main May 20, 2024
106 checks passed
@fcx-mrogez fcx-mrogez deleted the feat-expose-more-ryuk-config-via-env branch May 20, 2024 09:29
mdelapenya added a commit to bearrito/testcontainers-go that referenced this pull request Jun 11, 2024
* main: (48 commits)
  Fix race condition when looking up reaper (ryuk) container (testcontainers#2508)
  chore: bring golangci-lint back (testcontainers#2571)
  docs(compose): Fix typo docker compose docs (testcontainers#2565)
  Handle error properly during port forwarding initialization. (testcontainers#2550)
  chore: pin vearch version (testcontainers#2568)
  feat: add vearch module (testcontainers#2560)
  chore: run tests against latest Docker engine, nightly (testcontainers#2566)
  chore(deps): bump mkdocs-include-markdown-plugin from 6.0.4 to 6.0.7 (testcontainers#2562)
  Fix network accessor for port-forwarding feature (testcontainers#2551)
  --- (testcontainers#2549)
  fix: update search bar eval in mkdocs (testcontainers#2547)
  docs: improve contributing docs for code snippets (testcontainers#2546)
  chore: use a virtualenv for working with the docs site (testcontainers#2545)
  docs: document test session semantics (testcontainers#2544)
  feat(ryuk): allow to configure ryuk timeouts using env variables (testcontainers#2541)
  docs: fix CircleCI docs (testcontainers#2539)
  fix: add import to module generation (testcontainers#2537)
  chore: prepare for next minor development cycle (0.32.0)
  chore: use new version (v0.31.0) in modules and examples
  feat(mongodb): add replica set support via opts (testcontainers#2469)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New functionality or new behaviors on the existing one
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants