Skip to content

[mysql] move validation to shared for reuse #3150

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.

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
merged 3 commits into from
Jul 7, 2025
Merged

Conversation

heavycrystal
Copy link
Contributor

No description provided.

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR centralizes MySQL validation routines into a shared package and updates the MySQL connector to leverage these shared functions.

  • Extract MySQL version and binlog validation into flow/shared/mysql/validation.go
  • Refactor flow/connectors/mysql/validate.go to call shared validation, remove duplicated logic, and incorporate retry logic
  • Update imports and error handling to use the new shared package

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
flow/shared/mysql/validation.go New shared MySQL validation functions (version, flavor, binlog)
flow/connectors/mysql/validate.go Connector refactor to use shared validation and withRetries API
Comments suppressed due to low confidence (2)

flow/shared/mysql/validation.go:17

  • These new exported validation functions lack accompanying unit tests. Consider adding tests for version comparison, flavor validation, and each binlog settings checker to ensure correct behavior across MySQL/MariaDB versions.
func CompareServerVersion(conn *client.Conn, version string) (int, error) {

flow/shared/mysql/validation.go:17

  • Add Go doc comments for each exported function (e.g., // CompareServerVersion compares the server version to the target version and returns an integer result.) to improve maintainability and generated documentation.
func CompareServerVersion(conn *client.Conn, version string) (int, error) {

if errors.As(err, &mErr) && mErr.Code == mysql.ER_NO_SUCH_TABLE || mErr.Code == mysql.ER_TABLEACCESS_DENIED_ERROR {
// Table doesn't exist, which means this is not RDS/Aurora
logger.Warn("mysql.rds_configuration table does not exist, skipping Aurora/RDS binlog retention check",
slog.Any("error", err))
Copy link
Preview

Copilot AI Jul 6, 2025

Choose a reason for hiding this comment

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

Passing slog.Any("error", err) into a log.Logger may not render as expected. Use key/value pairs directly: logger.Warn("...", "error", err).

Suggested change
slog.Any("error", err))
"error", err)

Copilot uses AI. Check for mistakes.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@heavycrystal heavycrystal enabled auto-merge (squash) July 7, 2025 15:58
@heavycrystal heavycrystal merged commit 40b6e73 into main Jul 7, 2025
13 of 14 checks passed
@heavycrystal heavycrystal deleted the mysql-shared-validate branch July 7, 2025 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants