Skip to content

Conversation

@mdelapenya
Copy link
Member

What does this PR do?

Use the Run function in weaviate

Why is it important?

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

Related issues

This commit migrates the weaviate 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, WithExposedPorts, WithEnv, WithWaitStrategy
- Multiple wait strategies for HTTP and gRPC ports

Tests: 7 tests, 78.6% coverage

Ref: testcontainers#3174

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

Co-Authored-By: Claude <[email protected]>
@mdelapenya mdelapenya added the chore Changes that do not impact the existing functionality label Oct 9, 2025
@mdelapenya mdelapenya requested a review from a team as a code owner October 9, 2025 15:12
@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 a4f70e3
🔍 Latest deploy log https://app.netlify.com/projects/testcontainers-go/deploys/68e7d63c4c1c930008826970
😎 Deploy Preview https://deploy-preview-3442--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
    • None.
  • Refactor
    • Simplified container initialization using a consolidated, option-based setup for improved maintainability.
    • Preserved existing startup and wait behavior to avoid runtime changes.
    • Streamlined container handling and mapping to internal types.
    • Improved error messaging when container startup fails for clearer diagnostics.
  • Bug Fixes
    • None.
  • Tests
    • None.
  • Chores
    • No public API changes.

Walkthrough

Refactors the Weaviate module to construct container configuration using functional options and invokes testcontainers.Run. Removes manual GenericContainerRequest/GenericContainer usage, updates error text, preserves wait strategy, and adapts container handling to the Run return value.

Changes

Cohort / File(s) Summary of changes
Weaviate module run refactor
modules/weaviate/weaviate.go
Replaced manual GenericContainerRequest/GenericContainer flow with option-based testcontainers.Run using WithCmd, WithExposedPorts, WithEnv, and WithWaitStrategy; appended user opts; switched to handling ctr from Run; updated error message to "run weaviate: %w".

Sequence Diagram(s)

sequenceDiagram
    autonumber
    actor Caller
    participant Weaviate as Weaviate Module
    participant TC as testcontainers.Run
    participant Runtime as Container Runtime

    Caller->>Weaviate: Run(ctx, image, opts...)
    Weaviate->>Weaviate: Build moduleOpts (WithCmd, WithExposedPorts, WithEnv, WithWaitStrategy)
    Weaviate->>Weaviate: Append user-provided opts
    Weaviate->>TC: Run(ctx, image, moduleOpts...)
    TC->>Runtime: Create & start container
    Runtime-->>TC: Container handle or error
    alt success
        TC-->>Weaviate: ctr
        Weaviate->>Weaviate: Map ctr -> WeaviateContainer
        Weaviate-->>Caller: WeaviateContainer
    else failure
        TC-->>Weaviate: error
        Note right of Weaviate: Return error "run weaviate: %w"
        Weaviate-->>Caller: error
    end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • stevenh

Poem

I nudge the knobs with whiskered grace,
Options lined in tidy place—
WithCmd, WithEnv, ports in a row,
Then Run we hop, off we go!
No more generic maze to roam,
A cleaner path to container-home.
(\_\/) 🥕

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title Check ✅ Passed The title clearly and concisely conveys the main change of switching to the Run function in the weaviate module, matching the pull request’s primary intent without extraneous details.
Description Check ✅ Passed The description accurately outlines the pull request’s purpose of using the Run function and its rationale for migrating to the new API, and it references the related issue, so it directly relates to the changeset.
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 a4f70e3.

📒 Files selected for processing (1)
  • modules/weaviate/weaviate.go (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
modules/weaviate/weaviate.go (3)
options.go (5)
  • ContainerCustomizer (22-24)
  • WithCmd (462-467)
  • WithExposedPorts (454-459)
  • WithEnv (75-85)
  • WithWaitStrategy (366-368)
wait/host_port.go (1)
  • ForListeningPort (67-69)
wait/http.go (1)
  • ForHTTP (149-151)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: lint (modules/weaviate) / lint: modules/weaviate
🔇 Additional comments (3)
modules/weaviate/weaviate.go (3)

30-42: LGTM! Clean refactoring to functional options.

The migration to the functional options pattern improves code organization and aligns with the new testcontainers API. All container configuration is preserved: command arguments, exposed ports, environment variables, and wait strategies.


44-48: LGTM! Correct implementation with defensive nil check.

The call to testcontainers.Run correctly merges module defaults with user-provided options. The defensive nil check before wrapping the container is good practice, and returning the container alongside any error allows callers to perform cleanup if needed.


51-51: LGTM! More descriptive error message.

The updated error message "run weaviate: %w" is more specific and provides better context than the generic "generic container: %w".


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 88f1499 into testcontainers:main Oct 9, 2025
17 checks passed
@mdelapenya mdelapenya deleted the use-run-claude-weaviate branch October 9, 2025 15:47
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