Extend Provider interface with lifecycle methods#44
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdded four sandbox lifecycle methods ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Pull request overview
This PR extends the providers.Provider abstraction to support sandbox lifecycle operations (binary discovery + create/start/stop), laying groundwork for additional providers (e.g., ProxySQL) while keeping the existing MySQL provider behavior unchanged via stub methods.
Changes:
- Added
SandboxConfig/SandboxInfotypes and extended theProviderinterface withFindBinary,CreateSandbox,StartSandbox, andStopSandbox. - Added stub implementations of the new lifecycle methods to
MySQLProvider. - Updated the provider registry unit test mock to satisfy the expanded interface.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| providers/provider.go | Introduces sandbox config/info structs and expands the Provider interface with lifecycle methods. |
| providers/mysql/mysql.go | Implements the new interface methods on MySQLProvider as stubs (currently returning “not yet migrated” errors). |
| providers/provider_test.go | Extends the mock provider used in registry tests to implement the new interface methods. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func (p *MySQLProvider) CreateSandbox(config providers.SandboxConfig) (*providers.SandboxInfo, error) { | ||
| return nil, fmt.Errorf("MySQLProvider.CreateSandbox: use sandbox package directly (not yet migrated)") | ||
| } |
There was a problem hiding this comment.
This stub returns fmt.Errorf with a constant string and no formatting args; staticcheck (S1028) flags this. Prefer errors.New(...) for constant strings, or a shared sentinel error that you wrap with %w for caller detection.
| func (p *MySQLProvider) StartSandbox(dir string) error { | ||
| return fmt.Errorf("MySQLProvider.StartSandbox: use sandbox package directly (not yet migrated)") | ||
| } |
There was a problem hiding this comment.
fmt.Errorf is used here with a constant string and no formatting args; staticcheck (S1028) will flag it in CI. Use errors.New for constant errors (or wrap a sentinel error with %w).
| func (p *MySQLProvider) StopSandbox(dir string) error { | ||
| return fmt.Errorf("MySQLProvider.StopSandbox: use sandbox package directly (not yet migrated)") | ||
| } |
There was a problem hiding this comment.
Same as above: fmt.Errorf with a constant string and no formatting args is flagged by staticcheck (S1028). Prefer errors.New(...) (or wrap a shared sentinel error with %w).
| func (p *MySQLProvider) FindBinary(version string) (string, error) { | ||
| return "", fmt.Errorf("MySQLProvider.FindBinary: use sandbox package directly (not yet migrated)") | ||
| } |
There was a problem hiding this comment.
These stub methods use fmt.Errorf with a constant string and no formatting args. staticcheck (S1028) recommends using errors.New for constant error strings (or define a sentinel error and wrap it with %w). This will fail the repo's golangci-lint CI job as-is.
Summary
SandboxConfigandSandboxInfotypes toproviders/provider.goProviderinterface withFindBinary,CreateSandbox,StartSandbox, andStopSandboxmethodsMySQLProviderthat return "not yet migrated" errorsTest plan
go test ./providers/... -v— all 7 tests passgo build -o dbdeployer .— builds successfullyCloses #36
Summary by CodeRabbit
Release Notes
SandboxConfigandSandboxInfotypes to standardize sandbox configuration and operational state across providers.