Skip to content

Add MySQLProvider and register at startup #33

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.

Sign up for GitHub

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

Merged
renecannao merged 2 commits into from
Mar 24, 2026
Merged

Add MySQLProvider and register at startup #33

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b44f5ee
feat: add MySQLProvider implementing Provider interface
renecannao Mar 24, 2026
6d92899
feat: register MySQLProvider at startup via DefaultRegistry
renecannao Mar 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions cmd/root.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ import (
"github.com/ProxySQL/dbdeployer/defaults"
"github.com/ProxySQL/dbdeployer/downloads"
"github.com/ProxySQL/dbdeployer/globals"
"github.com/ProxySQL/dbdeployer/providers"
mysqlprovider "github.com/ProxySQL/dbdeployer/providers/mysql"
"github.com/ProxySQL/dbdeployer/sandbox"
)

Expand Down Expand Up @@ -143,6 +145,9 @@ func customizeFlags(cmd *cobra.Command, cmdName string) {
}

func init() {
if err := mysqlprovider.Register(providers.DefaultRegistry); err != nil {
panic(fmt.Sprintf("failed to register MySQL provider: %v", err))
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using panic in init() will produce a stack trace if registration ever fails (e.g., duplicate registration), which is a user-facing behavior change compared to the rest of the CLI error handling (which typically prints a message and exits). Consider switching to a controlled exit path (e.g., common.Exitf(1, ...) or deferring provider registration to Execute() where errors can be returned/printed cleanly).

Suggested change
panic(fmt.Sprintf("failed to register MySQL provider: %v", err))
common.Exitf(1, "failed to register MySQL provider: %v", err)

Copilot uses AI. Check for mistakes.
}
Comment on lines +148 to +150
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using panic for error handling during startup can be jarring for users of a CLI tool as it produces a stack trace. A more user-friendly approach is to print a clean error message to standard error and exit with a non-zero status code. This is also more consistent with error handling patterns seen elsewhere in the project.

Suggested change
if err := mysqlprovider.Register(providers.DefaultRegistry); err != nil {
panic(fmt.Sprintf("failed to register MySQL provider: %v", err))
}
if err := mysqlprovider.Register(providers.DefaultRegistry); err != nil {
fmt.Fprintf(os.Stderr, "failed to register MySQL provider: %v\n", err)
os.Exit(1)
}

cobra.OnInitialize(checkDefaultsFile)
rootCmd.CompletionOptions.DisableDefaultCmd = true
rootCmd.PersistentFlags().StringVar(&defaults.CustomConfigurationFile, globals.ConfigLabel, defaults.ConfigurationFile, "configuration file")
Expand Down
37 changes: 37 additions & 0 deletions providers/mysql/mysql.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
package mysql

import (
"fmt"
"strings"

"github.com/ProxySQL/dbdeployer/providers"
)

const ProviderName = "mysql"

type MySQLProvider struct{}

func NewMySQLProvider() *MySQLProvider {
return &MySQLProvider{}
}

func (p *MySQLProvider) Name() string { return ProviderName }

func (p *MySQLProvider) ValidateVersion(version string) error {
parts := strings.Split(version, ".")
if len(parts) < 2 {
return fmt.Errorf("invalid MySQL version format: %q (expected X.Y or X.Y.Z)", version)
}
return nil
Comment on lines +20 to +25
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ValidateVersion currently only checks that the string has at least one dot, so many clearly invalid versions are accepted (e.g., "8", "8.0.", "8..0", "8.0.0.1", "8.x"). This makes the validation misleading and can allow bad inputs to propagate to later version/feature logic. Consider enforcing a numeric X.Y or X.Y.Z format (reject empty components and >3 segments), e.g. via a regexp or by parsing each component as an int after strings.TrimSpace.

Copilot uses AI. Check for mistakes.
}
Comment on lines +20 to +26
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The current version validation only checks for the presence of at least one dot (.) in the version string. It doesn't ensure that the version components are numeric. This could lead to accepting invalid versions like "a.b" as valid. The validation should be strengthened to parse each component and verify it's a number. Note that you'll need to import the strconv package for this change.

func (p *MySQLProvider) ValidateVersion(version string) error {
	parts := strings.Split(version, ".")
	if len(parts) < 2 {
		return fmt.Errorf("invalid MySQL version format: %q (expected X.Y or X.Y.Z)", version)
	}
	for _, part := range parts {
		if _, err := strconv.Atoi(part); err != nil {
			return fmt.Errorf("invalid numeric part in version string %q: %s", version, part)
		}
	}
	return nil
}


func (p *MySQLProvider) DefaultPorts() providers.PortRange {
return providers.PortRange{
BasePort: 3306,
PortsPerInstance: 3, // main + mysqlx + admin
}
}

func Register(reg *providers.Registry) error {
return reg.Register(NewMySQLProvider())
}
47 changes: 47 additions & 0 deletions providers/mysql/mysql_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package mysql

import (
"testing"

"github.com/ProxySQL/dbdeployer/providers"
)

func TestMySQLProviderName(t *testing.T) {
p := NewMySQLProvider()
if p.Name() != "mysql" {
t.Errorf("expected 'mysql', got %q", p.Name())
}
}

func TestMySQLProviderValidateVersion(t *testing.T) {
p := NewMySQLProvider()
tests := []struct {
version string
wantErr bool
}{
{"8.4.4", false},
{"9.1.0", false},
{"5.7", false},
{"invalid", true},
Comment on lines +22 to +25
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The test suite for ValidateVersion can be improved by adding cases that test for non-numeric version components (e.g., "5.a") and empty components (e.g., from a trailing dot like "5.7."). This will help ensure the validation logic is robust against a wider range of invalid inputs, especially after strengthening the validation logic as suggested in mysql.go.

		{"8.4.4", false},
		{"9.1.0", false},
		{"5.7", false},
		{"invalid", true},
		{"5.a", true},
		{"a.5", true},
		{"5.7.", true},

}
for _, tt := range tests {
err := p.ValidateVersion(tt.version)
if (err != nil) != tt.wantErr {
t.Errorf("ValidateVersion(%q) error = %v, wantErr %v", tt.version, err, tt.wantErr)
}
}
Comment on lines +16 to +32
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ValidateVersion table test currently only covers one invalid case (no dots). Given the provider is meant to validate version strings, add cases that assert rejection of malformed-but-dotted inputs (e.g. trailing dot, empty component, extra component, non-numeric component) and acceptance of X.Y (e.g. "8.0"). This will prevent regressions once ValidateVersion is tightened.

Copilot uses AI. Check for mistakes.
}

func TestMySQLProviderRegister(t *testing.T) {
reg := providers.NewRegistry()
if err := Register(reg); err != nil {
t.Fatalf("Register failed: %v", err)
}
p, err := reg.Get("mysql")
if err != nil {
t.Fatalf("Get failed: %v", err)
}
if p.Name() != "mysql" {
t.Errorf("expected 'mysql', got %q", p.Name())
}
}
Loading