Add MySQL 8.4 LTS and 9.x Innovation release support#16
Add MySQL 8.4 LTS and 9.x Innovation release support#16renecannao wants to merge 4 commits intomasterfrom
Conversation
- Update go.mod minimum Go version from 1.18 to 1.22 - Remove obsolete //go:build go1.16 and // +build go1.16 constraints from downloads/tarball_registry.go (always satisfied with Go 1.22+) - Update direct dependencies to latest Go 1.22-compatible versions: - github.com/spf13/cobra v1.4.0 -> v1.8.1 - github.com/spf13/pflag v1.0.5 -> v1.0.6 - github.com/stretchr/testify v1.8.0 -> v1.9.0 - github.com/go-sql-driver/mysql v1.4.1 -> v1.7.1 - github.com/dustin/go-humanize v1.0.0 -> v1.0.1 - github.com/rogpeppe/go-internal v1.9.0 -> v1.12.0 - github.com/alexeyco/simpletable v0.0.0 -> v1.0.0 - github.com/araddon/dateparse updated - github.com/nightlyone/lockfile v0.0.0 -> v1.0.0 - golang.org/x/exp updated to Jun 2024 - golang.org/x/term v0.13.0 -> v0.25.0 - golang.org/x/text v0.13.0 -> v0.19.0 - golang.org/x/net v0.17.0 -> v0.30.0 (fixes GO-2024-3333) - github.com/xi2/xz left pinned (unmaintained since 2017, tech debt) - github.com/pkg/errors v0.9.1 unchanged (already latest) - Note: go-sql-driver/mysql latest (v1.9.x) requires Go 1.24+; pinned at v1.7.1 for Go 1.22 compatibility
Add a conservative golangci-lint config enabling errcheck, govet, ineffassign, staticcheck, and unused linters. Fix two real bugs caught by staticcheck: - SA1000: invalid regex syntax (?sg) in ts/commands.go (Go doesn't support 'g' flag) - SA1006: fmt.Errorf wrapping fmt.Sprintf unnecessarily in ops/downloads.go
There was a problem hiding this comment.
Pull request overview
This PR extends dbdeployer’s remote tarball/version-guessing support to recognize newer MySQL release trains (8.4 LTS and 9.x Innovation), and updates related metadata/configuration across the repo.
Changes:
- Extend MySQL version lists for remote downloads and guessing (8.4, 9.0–9.2) and add a test asserting the new guess-allowed versions.
- Bump the embedded tarball registry
DbdeployerVersionto 1.74.0. - Update tooling/docs (Go version bump + dependency upgrades, README/CONTRIBUTING updates, add golangci-lint config) and minor error/regex adjustments.
Reviewed changes
Copilot reviewed 11 out of 12 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
downloads/remote_tarballs.go |
Adds 8.4 and 9.x entries to downloadsSettings for remote tarball discovery. |
downloads/guess_latest.go |
Expands allowedGuessVersions to include 8.4 and 9.0–9.2. |
downloads/remote_registry_test.go |
Adds a test to confirm new guess-allowed versions are recognized. |
downloads/tarball_list.json |
Updates DbdeployerVersion metadata to 1.74.0. |
downloads/tarball_registry.go |
Removes the legacy build tags from the embed-based registry loader. |
ops/downloads.go |
Simplifies error construction when tarball lookup/guessing fails. |
ts/commands.go |
Adjusts regex used to surface “ERROR” matches from sandbox log files. |
go.mod / go.sum |
Bumps Go version to 1.22 and updates a set of dependencies accordingly. |
README.md |
Updates project links/badges and adds “New Maintainer” section. |
CONTRIBUTING.md |
Adds development setup notes (Go 1.22+, golangci-lint) and updates issue link. |
.golangci.yml |
Adds golangci-lint configuration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
|
|
||
| var allowedGuessVersions = []string{"5.7", "8.0"} | ||
| var allowedGuessVersions = []string{"5.7", "8.0", "8.4", "9.0", "9.1", "9.2"} |
There was a problem hiding this comment.
Now that guessing is enabled for 8.4/9.x, the guessing code path still hard-codes the Linux extension override only for shortVersion=="8.0" (else it defaults to .tar.gz). Add coverage and/or logic to ensure guessed filenames/URLs use the correct extension and naming conventions for 8.4 and 9.x, otherwise guess-latest may generate URLs that don’t exist.
| var allowedGuessVersions = []string{"5.7", "8.0", "8.4", "9.0", "9.1", "9.2"} | |
| // Only include versions for which the guessing logic (including extensions and naming) | |
| // is known to produce valid URLs. | |
| var allowedGuessVersions = []string{"5.7", "8.0"} |
| [DBdeployer](https://github.com/ProxySQL/dbdeployer) is a tool that deploys MySQL database servers easily. | ||
| This is a port of [MySQL-Sandbox](https://github.com/datacharmer/mysql-sandbox), originally written in Perl, and re-designed from the ground up in [Go](https://golang.org). See the [features comparison](https://github.com/datacharmer/dbdeployer/blob/master/docs/features.md) for more detail. |
There was a problem hiding this comment.
README now points to the ProxySQL-maintained repo, but the “features comparison” link still points to datacharmer/dbdeployer. Consider switching it to a relative link (e.g., docs/features.md) or updating to the new repo so it doesn’t send users to the legacy project.
| Versions: []string{"7.6", "8.0", "8.4", "9.0"}, | ||
| }, | ||
| TtMysql: { | ||
| Flavor: "mysql", | ||
| NameInFile: "mysql", | ||
| NameInUrl: "mysql", | ||
| DownloadDir: "MySQL-VERSION", | ||
| Versions: []string{"5.7", "8.0"}, | ||
| Versions: []string{"5.7", "8.0", "8.4", "9.0", "9.1", "9.2"}, | ||
| }, | ||
| TtShell: { | ||
| Flavor: "shell", | ||
| NameInFile: "mysql-shell", | ||
| NameInUrl: "shell", | ||
| DownloadDir: "MySQL-Shell-VERSION", | ||
| Versions: []string{"8.0"}, | ||
| Versions: []string{"8.0", "8.4", "9.0"}, | ||
| }, |
There was a problem hiding this comment.
The new MySQL Innovation versions are only partially added: cluster and shell settings include 9.0 but omit 9.1/9.2, which conflicts with the PR description/issue acceptance criteria (support 9.0–9.2 across mysql/cluster/shell). Add the missing versions here or clarify why those tarballs are intentionally unsupported.
| hasError := strings.Contains(string(contents), "ERROR") | ||
| if neg && hasError { | ||
| reLines := regexp.MustCompile(`(?sg)(^.*ERROR.*)`) | ||
| reLines := regexp.MustCompile(`(?s)(^.*ERROR.*)`) |
There was a problem hiding this comment.
The regex used to extract ERROR lines is still not doing what the code implies: (?s)(^.*ERROR.*) makes ^ match only the start of the whole file and . match newlines, so FindAll will typically return a single large match (not the individual lines with ERROR). Use multiline mode (e.g., (?m)^.*ERROR.*$) or split by lines and filter, so the failure output is actionable and bounded.
| reLines := regexp.MustCompile(`(?s)(^.*ERROR.*)`) | |
| reLines := regexp.MustCompile(`(?m)^.*ERROR.*$`) |
| if err != nil { | ||
| return downloads.TarballDescription{}, fmt.Errorf(fmt.Sprintf("Error getting version %s (%s-%s)[minimal: %v - newest: %v - guess: %v]: %s", | ||
| version, flavor, OS, minimal, newest, guessLatest, err)) | ||
| return downloads.TarballDescription{}, fmt.Errorf("Error getting version %s (%s-%s)[minimal: %v - newest: %v - guess: %v]: %s", |
There was a problem hiding this comment.
This returned error string is capitalized ("Error getting version...") while most other errors in this package are lowercase. Prefer starting errors with lowercase (e.g., "error getting version...") so callers can wrap/compose errors consistently.
| return downloads.TarballDescription{}, fmt.Errorf("Error getting version %s (%s-%s)[minimal: %v - newest: %v - guess: %v]: %s", | |
| return downloads.TarballDescription{}, fmt.Errorf("error getting version %s (%s-%s)[minimal: %v - newest: %v - guess: %v]: %s", |
| module github.com/datacharmer/dbdeployer | ||
|
|
||
| go 1.18 | ||
| go 1.22 |
There was a problem hiding this comment.
Bumping the module to go 1.22 will break CI/build environments still using Go 1.18 (the current GitHub Actions workflow sets go-version: ^1.18). Please update the CI workflow/tooling to use Go >= 1.22 (or revert this bump if it’s not intended in this PR).
| go 1.22 | |
| go 1.18 |
|
@gemini-code-assist review |
|
@claude review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds support for MySQL 8.4 LTS and 9.x innovation releases by updating version lists in downloads/guess_latest.go and downloads/remote_tarballs.go. A new test, TestNewMySQLVersionsRecognized, is included to verify these changes. The pull request also updates project dependencies, bumps the Go version to 1.22, and introduces a .golangci.yml file for linting. Minor code refactorings are present in ops/downloads.go and ts/commands.go. The changes appear correct and align with the PR's objective. As a point of feedback for future improvements, while documentation links have been updated to the new ProxySQL/dbdeployer repository, the Go module path in go.mod still refers to github.com/datacharmer/dbdeployer. Updating the module path would improve consistency across the project.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthroughAdded Go linting configuration and development setup requirements, upgraded Go toolchain from 1.22 with dependency updates, expanded MySQL version support for 8.4 and 9.x releases across configuration files, updated project metadata to reflect new maintainer, and applied minor code refinements in error handling and regex patterns. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.11.3)Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions 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
🧹 Nitpick comments (3)
ts/commands.go (1)
63-65: Regex currently captures the whole log, not individualERRORlines.At Line 63,
(?s)(^.*ERROR.*)is greedy over the full file, so the failure output at Line 65 can become the entire log instead of just matching lines. Prefer multiline line-by-line matching for clearer diagnostics.Suggested patch
- reLines := regexp.MustCompile(`(?s)(^.*ERROR.*)`) - errorLines := reLines.FindAll(contents, -1) - ts.Fatalf("ERRORs found in %s (%s)\n", logFile, errorLines) + reLines := regexp.MustCompile(`(?m)^.*ERROR.*$`) + errorLines := reLines.FindAllString(string(contents), -1) + ts.Fatalf("ERRORs found in %s:\n%s\n", logFile, strings.Join(errorLines, "\n"))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ts/commands.go` around lines 63 - 65, The regex used in regexp.MustCompile(`(?s)(^.*ERROR.*)`) is greedy and captures the entire file; change it to match individual ERROR lines by using a multiline, non-greedy pattern (e.g., enable ^ and $ per-line) so reLines = regexp.MustCompile(`(?m)^[^\n]*ERROR[^\n]*$`) (or equivalent) and then use reLines.FindAll(contents, -1) as before; update the ts.Fatalf call that prints errorLines to ensure it prints the slice of matched lines from errorLines for clearer diagnostics (references: reLines, regexp.MustCompile, errorLines, ts.Fatalf).downloads/remote_registry_test.go (1)
143-149: Add negative assertions to harden this guardrail.Lines 143-149 only test allowed values. Add at least one disallowed version check so accidental broadening is caught.
Suggested test hardening
func TestNewMySQLVersionsRecognized(t *testing.T) { versions := []string{"8.4", "9.0", "9.1", "9.2"} for _, v := range versions { result := isAllowedForGuessing(v) compare.OkEqualBool(fmt.Sprintf("version %s allowed for guessing", v), result, true, t) } + disallowed := []string{"9.3", "10.0"} + for _, v := range disallowed { + result := isAllowedForGuessing(v) + compare.OkEqualBool(fmt.Sprintf("version %s allowed for guessing", v), result, false, t) + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@downloads/remote_registry_test.go` around lines 143 - 149, TestNewMySQLVersionsRecognized currently only asserts allowed versions; add at least one negative assertion to ensure disallowed versions are rejected by isAllowedForGuessing. Update TestNewMySQLVersionsRecognized to include one or more inputs (e.g., "10.0" or "7.0") and assert that isAllowedForGuessing returns false for those values so accidental broadening of the guardrail is caught.README.md (1)
8-8: Consider simplifying "originally created and maintained."The phrase "originally created and maintained" contains a minor redundancy. Consider simplifying to "created and maintained" since "created" inherently implies the original creation. As per coding guidelines, this is flagged by static analysis as a style improvement.
✨ Suggested simplification
-This project was originally created and maintained by [Giuseppe Maxia](https://github.com/datacharmer) (datacharmer). It is now actively maintained by the [ProxySQL](https://github.com/ProxySQL) team, continuing Giuseppe's excellent work while expanding the project's capabilities. +This project was created and maintained by [Giuseppe Maxia](https://github.com/datacharmer) (datacharmer). It is now actively maintained by the [ProxySQL](https://github.com/ProxySQL) team, continuing Giuseppe's excellent work while expanding the project's capabilities.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@README.md` at line 8, Update the README sentence by removing the redundancy: replace the phrase "originally created and maintained by [Giuseppe Maxia] (datacharmer)" with "created and maintained by [Giuseppe Maxia] (datacharmer)"; specifically edit the line containing "This project was originally created and maintained by [Giuseppe Maxia](https://github.com/datacharmer) (datacharmer)." to use "created and maintained" instead.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@downloads/remote_tarballs.go`:
- Around line 64-79: The Versions lists for cluster/shell types are missing 9.1
and 9.2 which causes validateTarballRequest to reject those requests; update the
Versions slice for the cluster type (TtCluster) and the shell type (TtShell) to
include "9.1" and "9.2" (matching the existing TtMysql entries) so cluster and
shell requests for 9.1/9.2 are accepted.
---
Nitpick comments:
In `@downloads/remote_registry_test.go`:
- Around line 143-149: TestNewMySQLVersionsRecognized currently only asserts
allowed versions; add at least one negative assertion to ensure disallowed
versions are rejected by isAllowedForGuessing. Update
TestNewMySQLVersionsRecognized to include one or more inputs (e.g., "10.0" or
"7.0") and assert that isAllowedForGuessing returns false for those values so
accidental broadening of the guardrail is caught.
In `@README.md`:
- Line 8: Update the README sentence by removing the redundancy: replace the
phrase "originally created and maintained by [Giuseppe Maxia] (datacharmer)"
with "created and maintained by [Giuseppe Maxia] (datacharmer)"; specifically
edit the line containing "This project was originally created and maintained by
[Giuseppe Maxia](https://github.com/datacharmer) (datacharmer)." to use "created
and maintained" instead.
In `@ts/commands.go`:
- Around line 63-65: The regex used in regexp.MustCompile(`(?s)(^.*ERROR.*)`) is
greedy and captures the entire file; change it to match individual ERROR lines
by using a multiline, non-greedy pattern (e.g., enable ^ and $ per-line) so
reLines = regexp.MustCompile(`(?m)^[^\n]*ERROR[^\n]*$`) (or equivalent) and then
use reLines.FindAll(contents, -1) as before; update the ts.Fatalf call that
prints errorLines to ensure it prints the slice of matched lines from errorLines
for clearer diagnostics (references: reLines, regexp.MustCompile, errorLines,
ts.Fatalf).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 5e668ef2-a820-4608-b251-8c94d798679b
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (11)
.golangci.ymlCONTRIBUTING.mdREADME.mddownloads/guess_latest.godownloads/remote_registry_test.godownloads/remote_tarballs.godownloads/tarball_list.jsondownloads/tarball_registry.gogo.modops/downloads.gots/commands.go
💤 Files with no reviewable changes (1)
- downloads/tarball_registry.go
| Versions: []string{"7.6", "8.0", "8.4", "9.0"}, | ||
| }, | ||
| TtMysql: { | ||
| Flavor: "mysql", | ||
| NameInFile: "mysql", | ||
| NameInUrl: "mysql", | ||
| DownloadDir: "MySQL-VERSION", | ||
| Versions: []string{"5.7", "8.0"}, | ||
| Versions: []string{"5.7", "8.0", "8.4", "9.0", "9.1", "9.2"}, | ||
| }, | ||
| TtShell: { | ||
| Flavor: "shell", | ||
| NameInFile: "mysql-shell", | ||
| NameInUrl: "shell", | ||
| DownloadDir: "MySQL-Shell-VERSION", | ||
| Versions: []string{"8.0"}, | ||
| Versions: []string{"8.0", "8.4", "9.0"}, | ||
| }, |
There was a problem hiding this comment.
Cluster/Shell 9.x support is incomplete vs PR objective.
Line 64 and Line 78 only allow 9.0, while Line 71 allows 9.0, 9.1, 9.2 for mysql. This causes validateTarballRequest to reject cluster/shell requests for 9.1 and 9.2.
Proposed fix
TtCluster: {
Flavor: "ndb",
NameInFile: "mysql-cluster",
NameInUrl: "cluster",
DownloadDir: "MySQL-Cluster-VERSION",
- Versions: []string{"7.6", "8.0", "8.4", "9.0"},
+ Versions: []string{"7.6", "8.0", "8.4", "9.0", "9.1", "9.2"},
},
@@
TtShell: {
Flavor: "shell",
NameInFile: "mysql-shell",
NameInUrl: "shell",
DownloadDir: "MySQL-Shell-VERSION",
- Versions: []string{"8.0", "8.4", "9.0"},
+ Versions: []string{"8.0", "8.4", "9.0", "9.1", "9.2"},
},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@downloads/remote_tarballs.go` around lines 64 - 79, The Versions lists for
cluster/shell types are missing 9.1 and 9.2 which causes validateTarballRequest
to reject those requests; update the Versions slice for the cluster type
(TtCluster) and the shell type (TtShell) to include "9.1" and "9.2" (matching
the existing TtMysql entries) so cluster and shell requests for 9.1/9.2 are
accepted.
|
Changes already included in master via stacked branch merges. Closing. |
Summary
downloadsSettingsfor mysql, cluster, and shell tarball typesallowedGuessVersionsto include 8.4, 9.0, 9.1, 9.2DbdeployerVersionintarball_list.jsonto 1.74.0TestNewMySQLVersionsRecognizedto verify new versions are recognized for guessingTest plan
TestNewMySQLVersionsRecognizedpasses — all new versions recognizedTestFindOrGuessTarballByVersionFlavorOSpasses — existing behavior preservedTestMergeCollectionpasses — no regressionsCloses #5
Summary by CodeRabbit
New Features
Documentation
Chores