Skip to content

Conversation

@mdelapenya
Copy link
Member

What does this PR do?

Use the Run function in yugabytedb

Why is it important?

Migrate modules to the new API, improving consistency and leveraging the latest testcontainers functionality.

Related issues

This commit migrates the yugabytedb module to use the new testcontainers.Run() API.

The main changes are:
- Use testcontainers.Run() instead of testcontainers.GenericContainer()
- Convert to moduleOpts pattern with functional options
- Use WithCmd, WithWaitStrategy, WithExposedPorts, WithEnv
- Use Inspect after Run to retrieve actual env var values after user customizations
- Multiple wait strategies for logs and listening ports

Tests: 9 tests, 93.0% coverage

Ref: testcontainers#3174

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@mdelapenya mdelapenya requested a review from a team as a code owner October 9, 2025 15:34
@mdelapenya mdelapenya added the chore Changes that do not impact the existing functionality label Oct 9, 2025
@netlify
Copy link

netlify bot commented Oct 9, 2025

Deploy Preview for testcontainers-go ready!

Name Link
🔨 Latest commit 6114050
🔍 Latest deploy log https://app.netlify.com/projects/testcontainers-go/deploys/68e7d5f4eab0c70008cf2e0f
😎 Deploy Preview https://deploy-preview-3444--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 project configuration.

@coderabbitai
Copy link

coderabbitai bot commented Oct 9, 2025

Summary by CodeRabbit

  • New Features
    • Clearer, more specific error messages when starting or inspecting the service.
  • Bug Fixes
    • Ensures runtime environment values are reliably detected and applied after startup, preventing configuration mismatches.
    • Adds early checks to avoid null-related crashes during startup.
  • Refactor
    • Streamlines service startup by consolidating configuration into a single, simpler flow.
    • Removes redundant customization logic for improved reliability and maintainability.

Walkthrough

Refactors the YugabyteDB module to construct and start the container via testcontainers.Run with modular options, replaces manual ContainerRequest/GenericContainer usage, adds a post-start container inspect to read actual env values into fields, introduces targeted error messages, and includes early nil checks.

Changes

Cohort / File(s) Summary
YugabyteDB module: Run refactor
modules/yugabytedb/yugabytedb.go
Replace manual ContainerRequest/GenericContainer with optionized testcontainers.Run; consolidate image/cmd/ports/wait/env into With* options; add post-start Inspect to capture effective env and set Container fields; refine error messages; add nil checks; remove previous customization loop.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Dev as Caller
  participant YB as YugabyteDB Module
  participant TC as testcontainers.Run
  participant Docker as Docker Engine

  Dev->>YB: Run(ctx, req)
  rect rgba(200,230,255,0.3)
    note right of YB: Build moduleOpts (WithImage, WithCmd,<br/>WithExposedPorts, WithEnv, WithWait)
    YB->>TC: Run(ctx, moduleOpts...)
    TC->>Docker: Create+Start container
    Docker-->>TC: Container handle
    TC-->>YB: Container (c)
  end

  alt c == nil
    YB-->>Dev: error("run yugabytedb: ...")
  else Post-start inspect
    YB->>Docker: Inspect container (env)
    Docker-->>YB: Env key/values
    note right of YB: Map inspected env to Container fields
    YB-->>Dev: Container handle + populated fields
  end

  %% Error path on inspect
  opt Inspect fails
    YB-->>Dev: error("inspect yugabytedb: ...")
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

I tug my ears, refactor done,
Options hop—Run, then inspect—what fun!
Ports exposed, envs align,
Carrots mapped to fields so fine.
With gentler errs and cleaner flow,
This burrow’s ready—on we go! 🥕🐇

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The title clearly and concisely summarizes the primary change of migrating the yugabytedb module to use the Run function, follows conventional commit style, and provides enough context for a reviewer to understand the main intent.
Description Check ✅ Passed The description directly relates to the changeset by explaining what the PR does, why it is important, and linking to relevant issues, providing sufficient context for reviewers.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1c64a46 and 6114050.

📒 Files selected for processing (1)
  • modules/yugabytedb/yugabytedb.go (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
modules/yugabytedb/yugabytedb.go (3)
options.go (4)
  • ContainerCustomizer (22-24)
  • WithCmd (462-467)
  • WithWaitStrategy (366-368)
  • WithExposedPorts (454-459)
wait/log.go (1)
  • ForLog (118-120)
wait/host_port.go (1)
  • ForListeningPort (67-69)
🔇 Additional comments (1)
modules/yugabytedb/yugabytedb.go (1)

86-105: Solid env sync after user customizations

Great touch pulling the effective YSQL env values from Inspect; this keeps the module’s connection helpers aligned with any overrides users apply. 👍


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@mdelapenya mdelapenya merged commit 3852045 into testcontainers:main Oct 9, 2025
17 checks passed
@mdelapenya mdelapenya deleted the use-run-claude-yugabytedb branch October 9, 2025 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore Changes that do not impact the existing functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant