Skip to content

feat: mysql integration ci#23

Merged
cungminh2710 merged 2 commits into
mainfrom
minhc/mysql-ci
Mar 29, 2026
Merged

feat: mysql integration ci#23
cungminh2710 merged 2 commits into
mainfrom
minhc/mysql-ci

Conversation

@cungminh2710
Copy link
Copy Markdown
Contributor

What this PR for?

What did you do for validating the changes?

Anything else you want to add? (Optional)

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Mar 29, 2026

Greptile Summary

This PR adds a mysql-integration CI job to the GitHub Actions workflow and accompanies it with an ADR documenting the design decision. The new job follows the same pattern already established by the postgres-integration job: a service container, a health-check, Go setup, and a targeted go test invocation with a dialect-specific DSN.

Key points:

  • The job structure, Go version (1.26), timeout-minutes, and needs: [precommit] dependency are all consistent with the existing SQLite and Postgres jobs.
  • The mysql:8.4 service container is correctly configured with MYSQL_DATABASE, MYSQL_USER, MYSQL_PASSWORD, and MYSQL_ROOT_PASSWORD.
  • The RAIN_MYSQL_DSN DSN format (rain:rain@tcp(127.0.0.1:3306)/rain_test?parseTime=true) is correct for the go-sql-driver/mysql library.
  • The ADR is thorough and accurately reflects what was implemented.
  • One minor note: the health-check probe authenticates as the rain user; with MySQL 8.4's caching_sha2_password default this can occasionally timeout on the very first probe. Switching the probe to the root account is a common workaround.

Confidence Score: 5/5

Safe to merge — the new job is a clean incremental addition with no impact on existing jobs.

All findings are P2 style suggestions. The workflow change is additive, consistent with existing patterns, and has a supporting ADR. No correctness, data-integrity, or reliability issues were identified.

No files require special attention.

Important Files Changed

Filename Overview
.github/workflows/ci.yml Adds a mysql-integration job that mirrors the existing postgres-integration structure — service container, health check, Go setup, and test run via RAIN_MYSQL_DSN. One minor note: the health-check uses the non-root rain user, which can occasionally fail on cold-start with MySQL 8.4's caching_sha2_password default.
docs/adr/2026-03-29-mysql-integration-ci-design.md New ADR documenting the decision to add a dedicated MySQL CI job; well-structured and accurately reflects the implementation choices in the workflow file.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    PR[Push / Pull Request] --> PC[precommit job\nmake precommit]
    PC --> SQ[integration job\nSQLite Integration Tests]
    PC --> PG[postgres-integration job\nPostgres Integration Tests]
    PC --> MY[mysql-integration job\nMySQL Integration Tests NEW]

    MY --> SVC[mysql:8.4 service container\nrain_test DB · rain user]
    SVC -->|health-check passes| GO[Setup Go 1.26]
    GO --> TEST["go test -race ./pkg/rain\n-run '^TestMySQLIntegration'"]
Loading
Prompt To Fix All With AI
This is a comment left during a code review.
Path: .github/workflows/ci.yml
Line: 96

Comment:
**Consider using root for the health-check user**

MySQL 8.4 defaults to the `caching_sha2_password` authentication plugin for newly created users. On the very first health-check probe (before any authentication cache is populated), `mysqladmin ping` with a `caching_sha2_password` account over a plain TCP connection can occasionally fail because the full RSA key-exchange handshake is required.

Using `root` for the health-check avoids this cold-start issue entirely, since the root account created by `MYSQL_ROOT_PASSWORD` commonly uses `mysql_native_password` in the official image:

```suggestion
          --health-cmd="mysqladmin ping -h 127.0.0.1 -uroot -proot"
```

The `rain` user is still the correct credential to put in `RAIN_MYSQL_DSN` for the actual test run; this change only affects the liveness probe.

How can I resolve this? If you propose a fix, please make it concise.

Reviews (1): Last reviewed commit: "." | Re-trigger Greptile

Comment thread .github/workflows/ci.yml
@cungminh2710 cungminh2710 merged commit 3e86190 into main Mar 29, 2026
4 checks passed
@cungminh2710 cungminh2710 deleted the minhc/mysql-ci branch March 29, 2026 11:05
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.

1 participant