Skip to content

Update CI #1034

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 1 commit into from
May 8, 2025
Merged

Update CI #1034

merged 1 commit into from
May 8, 2025

Conversation

dveeden
Copy link
Collaborator

@dveeden dveeden commented May 6, 2025

  • Add ubuntu-latest
  • Add MySQL 9.x innovation release
  • Remove Go 1.22
  • Update MySQL 8.0 and 8.4 versions

@dveeden dveeden force-pushed the update_ci_2025_may branch 3 times, most recently from 6eb50bc to d35fa16 Compare May 7, 2025 05:17
@dveeden dveeden force-pushed the update_ci_2025_may branch 9 times, most recently from e93fcb7 to 95f592b Compare May 7, 2025 06:20
@dveeden dveeden marked this pull request as ready for review May 7, 2025 06:20
@dveeden
Copy link
Collaborator Author

dveeden commented May 7, 2025

I wanted to add tests with MariaDB, but wasn't able to get a service container with MariaDB running with binlogs enabled.

  • Adding arguments like --log-bin --binlog_format=ROW --server-id=1 isn't supported in service container configs
  • Adding a mariadb.cnf that is mounted on /etc/mysql/mariadb.cnf didn't seem to work.
  • Creating a custom container with the right config file seems overkill and a maintenance nightmare

@dveeden
Copy link
Collaborator Author

dveeden commented May 7, 2025

cc @alarbada @serprex

@dveeden dveeden force-pushed the update_ci_2025_may branch from 29494b1 to 95f592b Compare May 7, 2025 07:34
@dveeden
Copy link
Collaborator Author

dveeden commented May 7, 2025

I also tried this:

commit 29494b1aea4fd29e4e8d1cab179a67641913e10a (HEAD -> update_ci_2025_may, origin/update_ci_2025_may)
Author: Daniël van Eeden <[email protected]>
Date:   Wed May 7 09:30:18 2025 +0200

    Enable disabled tests

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index af153ff..06abb21 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -88,9 +88,9 @@ jobs:
       - name: Run tests
         run: |
           # separate test to avoid RESET MASTER conflict
-          # TODO: Fix "dump/" and "canal/": mysqldump tries to run SHOW MASTER STATUS on v8.4.0
           go test $(go list ./... | grep -v canal | grep -v dump)
-          # go test $(go list ./... | grep canal | grep -v dump)
+          go test $(go list ./... | grep canal)
+          go test $(go list ./... | grep dump)
 
   golangci:
     name: golangci

But that failed on MySQL 8.4 and 9.x. I'm not sure why. This might be for a different reason that the SHOW MASTER STATUS issue listed in the comment.

@serprex
Copy link
Contributor

serprex commented May 7, 2025

To get MariaDB in PeerDB's CI I had to have a step run docker directly: https://github.com/PeerDB-io/peerdb/blob/84fe9ce286a8c8cc03a6369e4b7ce4b2f7505d1c/.github/workflows/flow.yml#L125

Note maria seems to have MIXED as default binlog format

To deal with port conflicts I have maria & mysql tests run separately

@lance6716
Copy link
Collaborator

@dveeden After changing CI, you should also adjust "required CI" in the repo setting to align. Do you want me to help you adjust it?

@dveeden
Copy link
Collaborator Author

dveeden commented May 8, 2025

@lance6716 I might not have the permissions to do that.

Also, a required check did not run for this PR, so it can't be merged yet. This is because there is a required check for Go 1.22... which was removed in this PR

@dveeden dveeden merged commit 31aad20 into go-mysql-org:master May 8, 2025
36 checks passed
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.

3 participants