chore(dolt): use Run function #3405
Conversation
✅ Deploy Preview for testcontainers-go ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Summary by CodeRabbit
WalkthroughRefactors the Dolt module to use testcontainers.Run with assembled options, defers credential handling to post-start Inspect, adds default-credential helper (public deprecated + private), validates discovered credentials at runtime, and conditionally initializes database/user after container start. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Caller
participant Dolt as Dolt module
participant TC as testcontainers
participant C as Container
participant DB as Dolt DB init
Caller->>Dolt: Run(ctx, opts...)
Dolt->>Dolt: Build moduleOpts (WithExposedPorts, WithEnv, WithWaitStrategy)
Dolt->>Dolt: Append user opts + withDefaultCredentials
Dolt->>TC: Run(ctx, image, moduleOpts)
TC-->>Dolt: Container handle (C)
rect rgba(230,245,255,0.5)
note right of Dolt: Post-start credential discovery
Dolt->>C: Inspect -> env
Dolt->>Dolt: Derive user/password/database (default to root if missing)
end
alt user != root AND password == ""
Dolt-->>Caller: error (missing password for non-root user)
else createUser == true
rect rgba(240,255,240,0.6)
note over Dolt,DB: Conditional initialization
Dolt->>DB: Create database if missing
Dolt->>DB: Create user & grant permissions
end
Dolt-->>Caller: DoltContainer (with populated credentials)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
modules/dolt/dolt.go(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
modules/dolt/dolt.go (4)
modules/mysql/mysql.go (2)
WithDefaultCredentials(29-45)Run(54-105)options.go (5)
CustomizeRequestOption(28-28)ContainerCustomizer(22-24)WithExposedPorts(454-459)WithEnv(75-85)WithWaitStrategy(366-368)modules/consul/consul.go (1)
Run(61-83)container.go (1)
Container(41-73)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
modules/dolt/dolt.go (1)
106-108: Simplify redundant password checks.The condition
len(dc.password) == 0 && dc.password == ""is redundant since both checks are equivalent in Go. Consider simplifying to justdc.password == "".Apply this diff:
- if len(dc.password) == 0 && dc.password == "" && !strings.EqualFold(rootUser, dc.username) { + if dc.password == "" && !strings.EqualFold(rootUser, dc.username) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
modules/dolt/dolt.go(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
modules/dolt/dolt.go (4)
modules/mysql/mysql.go (2)
WithDefaultCredentials(29-45)Run(54-105)options.go (5)
CustomizeRequestOption(28-28)ContainerCustomizer(22-24)WithExposedPorts(454-459)WithEnv(75-85)WithWaitStrategy(366-368)modules/clickhouse/clickhouse.go (1)
Run(69-123)container.go (1)
Container(41-73)
⏰ 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). (2)
- GitHub Check: lint (modules/dolt) / lint: modules/dolt
- GitHub Check: Analyze (go)
🔇 Additional comments (4)
modules/dolt/dolt.go (4)
30-33: LGTM! Clean deprecation pattern.The deprecation and delegation to the private helper follows the same pattern used in the MySQL module and aligns with the PR objective to make this option internal.
35-47: LGTM! Appropriate credential handling for root.The private helper correctly removes credentials when using the root user, which is appropriate for Dolt's authentication model.
56-77: LGTM! Proper use of testcontainers.Run.The refactoring correctly uses
testcontainers.Runwith assembled options, following the pattern established in the ClickHouse module. The order of appending options (module defaults → user opts → WithDefaultCredentials) ensures proper credential handling.
79-104: LGTM! Credential discovery correctly handles root case.The post-start credential discovery properly inspects the container environment and correctly handles the root user case by clearing the password (line 102), which addresses the previous review comment. This ensures
ConnectionStringcalls will emit root with an empty password.
1 participant
What does this PR do?
Use Run function in the Dolt module, deprecating an option that should be internal.
Why is it important?
Migrate modules to the new API
Related issues