checker(dm): update MariaDB precheck instruction#12669
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Code Review
This pull request updates the warning instruction for MariaDB migrations to advise verifying compatibility instead of recommending a specific version upgrade, and adds a corresponding unit test. The review feedback suggests simplifying the warning instruction to remove redundant phrasing about MariaDB migration being experimental, and updating the test assertion accordingly.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if conn.IsMariaDB(value) { | ||
| err := NewWarn("Migrating from MariaDB is still experimental.") | ||
| err.Instruction = "It is recommended that you upgrade MariaDB to 10.1.2 or a later version." | ||
| err.Instruction = "It is recommended that you verify compatibility before performing data migration because migrating from MariaDB is still experimental. Otherwise data inconsistency or task exceptions might occur." |
There was a problem hiding this comment.
The instruction string repeats "because migrating from MariaDB is still experimental", which is redundant since the warning message itself is already "Migrating from MariaDB is still experimental.". Removing this clause makes the instruction more concise and direct.
| err.Instruction = "It is recommended that you verify compatibility before performing data migration because migrating from MariaDB is still experimental. Otherwise data inconsistency or task exceptions might occur." | |
| err.Instruction = "It is recommended that you verify compatibility before performing data migration. Otherwise data inconsistency or task exceptions might occur." |
| require.Equal(t, result.Instruction, | ||
| "It is recommended that you verify compatibility before performing data migration because migrating from MariaDB is still experimental. Otherwise data inconsistency or task exceptions might occur.") |
There was a problem hiding this comment.
Update the test assertion to match the simplified instruction string.
| require.Equal(t, result.Instruction, | |
| "It is recommended that you verify compatibility before performing data migration because migrating from MariaDB is still experimental. Otherwise data inconsistency or task exceptions might occur.") | |
| require.Equal(t, result.Instruction, | |
| "It is recommended that you verify compatibility before performing data migration. Otherwise data inconsistency or task exceptions might occur.") |
|
@GMHDBJD: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
What problem does this PR solve?
Issue Number: close #11734
What is changed and how it works?
DM reports a warning for MariaDB upstreams because MariaDB migration is still experimental, but the precheck instruction still suggested upgrading MariaDB to 10.1.2 or later. That suggestion is obsolete and unhelpful for newer MariaDB versions such as 11.4.2.
This PR updates the MariaDB version precheck instruction to recommend verifying compatibility before migration, and adds a regression test for the MariaDB instruction so it no longer mentions 10.1.2.
Check List
Tests
go test ./dm/pkg/checker ./dm/checker -count=1make fmtgit diff --checkQuestions
Will it cause performance regression or break compatibility?
No. This only changes the user-facing precheck instruction for MariaDB upstreams and adds a regression test.
Do you need to update user documentation, design documentation or monitoring documentation?
No.
Release note