ci: add MySQL 5.6 to sandbox-test matrix#90
Conversation
MySQL 5.6 has been supported by dbdeployer since inception, but CI didn't exercise it. Add 5.6.51 (the last 5.6 release) to the sandbox-test job covering single + master-slave replication, and update the docs so the supported-versions matrix reflects reality. MySQL 5.6 tarballs ship as .tar.gz built against glibc2.12, unlike 5.7+ which ship .tar.xz/glibc2.17, so the download/unpack steps branch on short version.
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 48 minutes and 15 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughExtended MySQL 5.6 support across testing and documentation. The integration test workflow now includes MySQL 5.6.51 with version-specific tarball handling logic. README and provider documentation updated to reflect 5.6 as a supported legacy version with specific topology constraints. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![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 updates the documentation to include MySQL 5.6 in the supported versions list and clarifies version requirements for various topologies. The review feedback suggests removing redundant 'unsupported' notes for the 5.6.x series to maintain consistency with other entries. Additionally, it is recommended to revert the minimum version for Single and Master-slave topologies to 'any', as the tool maintains compatibility with versions older than 5.6.
|
|
||
| | Series | Status | Topologies | | ||
| |--------|--------|-----------| | ||
| | 5.6.x | Legacy | single, replication (GTID 5.6.9+), fan-in/all-masters unsupported | |
There was a problem hiding this comment.
The note fan-in/all-masters unsupported is redundant in this table because the "Supported Topologies" table below already explicitly states that these topologies require at least version 5.7.9. Removing this note would make the row cleaner and more consistent with the 5.7.x row, which only lists supported features.
| | 5.6.x | Legacy | single, replication (GTID 5.6.9+), fan-in/all-masters unsupported | | |
| | 5.6.x | Legacy | single, replication (GTID 5.6.9+) | |
| | Single | `deploy single` | 5.6 | | ||
| | Master-slave | `--topology=master-slave` (default) | 5.6 | |
There was a problem hiding this comment.
Changing the minimum version from any to 5.6 for Single and Master-slave topologies is technically inaccurate. While this PR adds 5.6 to the official test matrix, dbdeployer continues to support much older versions (e.g., 5.5, 5.1, 5.0, and 4.1 as evidenced by the functional test suite in test/functional-test.sh). Since these topologies have no technical version floor within the tool's capabilities, any remains the most accurate description.
| | Single | `deploy single` | 5.6 | | |
| | Master-slave | `--topology=master-slave` (default) | 5.6 | | |
| | Single | deploy single | any | | |
| | Master-slave | --topology=master-slave (default) | any | |
![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 (2)
.github/workflows/integration_tests.yml (1)
53-79: Duplicated tarball-selection logic between download and unpack steps.The
SHORT_VERderivation andTARBALLbranching are duplicated verbatim in both "Download MySQL" (lines 53-60) and "Unpack MySQL" (lines 74-79). If the naming convention ever needs another special case (e.g., ARM, 5.5, or glibc variants), both places must be kept in sync. Consider computing once in the download step and exporting via$GITHUB_ENVso the unpack step can reuse it.♻️ Proposed refactor
- name: Download MySQL run: | SHORT_VER=$(echo "$MYSQL_VERSION" | grep -oP '^\d+\.\d+') - # MySQL 5.6 ships as .tar.gz built against glibc2.12; 5.7+ ships - # as .tar.xz built against glibc2.17. if [ "$SHORT_VER" = "5.6" ]; then TARBALL="mysql-${MYSQL_VERSION}-linux-glibc2.12-x86_64.tar.gz" else TARBALL="mysql-${MYSQL_VERSION}-linux-glibc2.17-x86_64.tar.xz" fi + echo "TARBALL=$TARBALL" >> "$GITHUB_ENV" + echo "SHORT_VER=$SHORT_VER" >> "$GITHUB_ENV" mkdir -p /tmp/mysql-tarball if [ ! -f "/tmp/mysql-tarball/$TARBALL" ]; then ... - name: Unpack MySQL run: | mkdir -p "$SANDBOX_BINARY" - SHORT_VER=$(echo "$MYSQL_VERSION" | grep -oP '^\d+\.\d+') - if [ "$SHORT_VER" = "5.6" ]; then - TARBALL="mysql-${MYSQL_VERSION}-linux-glibc2.12-x86_64.tar.gz" - else - TARBALL="mysql-${MYSQL_VERSION}-linux-glibc2.17-x86_64.tar.xz" - fi ./dbdeployer unpack "/tmp/mysql-tarball/$TARBALL" \ --sandbox-binary="$SANDBOX_BINARY"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/integration_tests.yml around lines 53 - 79, The tarball selection (SHORT_VER and TARBALL) is duplicated; compute SHORT_VER and TARBALL once in the "Download MySQL" step and export TARBALL (and optionally SHORT_VER) to the environment so the "Unpack MySQL" step can reuse them via $GITHUB_ENV; update the "Download MySQL" step to write TARBALL into $GITHUB_ENV after determining it, and remove the duplicated logic from the "Unpack MySQL" step so it simply reads the exported $TARBALL (and $SHORT_VER if needed).website/src/content/docs/providers/mysql.md (1)
88-88: Minor: "replication (GTID 5.6.9+)" could be misread as requiring GTID.Plain master-slave replication works on 5.6.x without GTID; GTID is only required when
--gtidis used (which needs 5.6.9+). Consider rewording to e.g.single, replication (GTID requires 5.6.9+); fan-in/all-masters unsupportedto avoid implying GTID is mandatory.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@website/src/content/docs/providers/mysql.md` at line 88, The table entry string "replication (GTID 5.6.9+)" can be misread as GTID being required; update that text to clarify GTID is optional and only required when using the --gtid flag and that GTID support needs 5.6.9+. Replace the current cell text with something like "single, replication (GTID requires 5.6.9+), fan-in/all-masters unsupported" so the meaning in the MySQL providers doc is unambiguous.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@website/src/content/docs/providers/mysql.md`:
- Around line 100-101: The table entry narrowing minimum MySQL version to "5.6"
for "deploy single" and "--topology=master-slave" appears accidental; revert
those two cells back to "any" (restore the previous wording for the `deploy
single` and `--topology=master-slave` rows) unless dropping pre-5.6 support is
intentional—if intentional, add a short note explaining the change and update
any related docs/tests to reflect the new minimum.
---
Nitpick comments:
In @.github/workflows/integration_tests.yml:
- Around line 53-79: The tarball selection (SHORT_VER and TARBALL) is
duplicated; compute SHORT_VER and TARBALL once in the "Download MySQL" step and
export TARBALL (and optionally SHORT_VER) to the environment so the "Unpack
MySQL" step can reuse them via $GITHUB_ENV; update the "Download MySQL" step to
write TARBALL into $GITHUB_ENV after determining it, and remove the duplicated
logic from the "Unpack MySQL" step so it simply reads the exported $TARBALL (and
$SHORT_VER if needed).
In `@website/src/content/docs/providers/mysql.md`:
- Line 88: The table entry string "replication (GTID 5.6.9+)" can be misread as
GTID being required; update that text to clarify GTID is optional and only
required when using the --gtid flag and that GTID support needs 5.6.9+. Replace
the current cell text with something like "single, replication (GTID requires
5.6.9+), fan-in/all-masters unsupported" so the meaning in the MySQL providers
doc is unambiguous.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 54bbdd17-3a2f-4b19-9b5c-cca3b6576168
📒 Files selected for processing (3)
.github/workflows/integration_tests.ymlREADME.mdwebsite/src/content/docs/providers/mysql.md
- Revert `Single` and `Master-slave` min-version cells to `any` in the Topologies table. Adding 5.6 to CI doesn't narrow the tool's actual floor — dbdeployer still works with 5.5 and earlier (ts/setup.go default list includes 5.5, functional tests cover older versions). - Simplify the new `5.6.x` row in the Supported Versions table. The GTID and topology-exclusion notes belong in the Topologies table that follows; the 5.7.x row doesn't annotate them either. - Dedupe the tarball-selection logic in integration_tests.yml by computing it once in a "Resolve tarball name" step and exporting via $GITHUB_ENV for Download and Unpack to consume.
1 participant
Summary
sandbox-testmatrix inintegration_tests.yml(single + master-slave replication).linux-glibc2.12-x86_64.tar.gz, 5.7+ keepslinux-glibc2.17-x86_64.tar.xz.Context: dbdeployer has supported MySQL 5.6 since forever (capability gates like
MinimumCrashSafeVersion = 5.6.2,MinimumGtidVersion = 5.6.9inglobals/globals.go), but CI never exercised it and the docs implied 5.7 was the floor.Test plan
Sandbox (5.6.51)job succeeds: unpack, deploy single, deploy replication,check_slaves,test_replication.Summary by CodeRabbit
Documentation
Tests