Skip to content
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.

Sign up for GitHub

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

Jump to bottom

CI: Update MariaDB versions #6426

Merged
merged 3 commits into from
Jun 8, 2024
Merged

CI: Update MariaDB versions #6426

merged 3 commits into from
Jun 8, 2024

Conversation

grooverdan
Copy link
Contributor

Q A
Type improvement
Fixed issues N/A

Summary

Improve the CI to cover newer versions of MariaDB 11.4+.

Also cover the quay.io/mariadb-foundation/mariadb-devel images from https://quay.io/repository/mariadb-foundation/mariadb-devel?tab=tags where they are updated with completed, reviewed and tested changes in the MariaDB server main branch.

These are completed features/bug fixes in the next server version. As such this is a good thing to test before the users get the final product.

ref: https://mariadb.org/new-service-quay-io-mariadb-foundation-mariadb-devel/

The test failures here are fixed by #6425.

@derrabus
Copy link
Member

derrabus commented Jun 7, 2024 •
edited
Loading

Thank you. We already have a very large CI matrix and I'd like to avoid adding more and more checks on top.

  • About adding 11.4 to the matrix: absolutely. In fact, if you submitted that change only as a separate PR, I'd merge it right away.
  • While you're at it, let's kick 11.0. It has just reached its EOL.
  • 11.5-rc: We usually wait for a stable release before adding a new image to the matrix. Given that there are not new features in 11.5 that we currently make use of, I'd rather not add 11.5 just yet.
  • About the devel images: I'm really not sold here. It feels like we're running the CI for other people software and I don't want to do that.

@derrabus
Copy link
Member

derrabus commented Jun 7, 2024

Looks like the CI has spotted an incompatibility with 11.4. 🙈

@grooverdan
Copy link
Contributor Author

Looks like the CI has spotted an incompatibility with 11.4. 🙈

The test failures here are fixed by #6425.

@grooverdan
Copy link
Contributor Author

Thank you. We already have a very large CI matrix an I'd like to avoid adding more and more checks on top.

* About adding 11.4 to the matrix: absolutely. In fact, if you submitted that change only as a separate PR, I'd merge it right away.

* While you're at it, let's kick 11.0. It has just reached its EOL.

easy.

* 11.5-rc: We usually wait for a stable release before adding a new image to the matrix. Given that there are not new features in 11.5 that we currently make use of, I'd rather not add 11.5 just yet.

Ack.

* About the devel images: I'm really not sold here. It feels like we're running the CI for other people software and I don't want to do that.

As #6425 shows, the way Doctrine use other people's software isn't 100% stable, occasionally assumptions are made, and broken.

When code reaches quay.io/mariadb-foundation/mariadb-devel, its been tested, though CI and reviewed. The only thing that will detect an incompatibility is another set of tests (or an end user which is what I'm trying to avoid). With a bug report, its probably even fixable in MariaDB before release. A case where this worked for another upstream connector is https://jira.mariadb.org/browse/MDEV-33592, and hopefully no mutual users even noticed.

Its your project obviously, I'll take your decision however it comes.

I'll incorporated the desired changes and rebase after #6425 so it looks really green before a merge.

@derrabus
Copy link
Member

derrabus commented Jun 7, 2024

Okay, let's split the PR into two:

  • One that does the obvious maintenance work: Add 11.4, drop 11.0. This one can be merged easily.
  • One that adds the dev images. We need further discussion with the other maintainer on whether we want this at all and what our strategy for testing against dev snapshots of databases.

@grooverdan
ci mariadb - add 11.4/remove 11.0 versions
846dc79
@grooverdan
ci mariadb - use 11.4, latest lts in includes
b1ade1f 
It the latest stable version so ensure it works.
@grooverdan
ci mariadb - healthcheck
a259450 
MariaDB containers except the 10.0 version have a healthcheck.sh script.
Use this to check if the container is ready.

mysqladmin ping has a problem that without the --protocol tcp it will return
healthy in the intialization phases where mariadb is running without networking.

By adding --protocol tcp the 10.0 instance can return healthy once a tcp
socket is listenting.
@derrabus derrabus mentioned this pull request Jun 8, 2024
Merged
derrabus added a commit that referenced this pull request Jun 8, 2024
@derrabus @grooverdan
CI MariaDB: add 11.4, remove 11.0 (#6432)
ac1b4fb 
Cherry-picked from #6426.

Co-authored-by: Daniel Black <[email protected]>
@derrabus
Copy link
Member

derrabus commented Jun 8, 2024

I've chery-picked the first commit to #6432.

@derrabus
Copy link
Member

derrabus commented Jun 8, 2024

@greg0ire @morozov What do you think about testing against MariaDB snapshots? Should we do that?

@derrabus derrabus changed the title CI: Update MariaDB versions and extend to in development versions CI: Update MariaDB versions Jun 8, 2024
@derrabus
Copy link
Member

derrabus commented Jun 8, 2024

I just noticed, you threw out all changes related to the dev snapshots. I'm merging as is, please submit a new PR where we can discuss including the dev versions.

@derrabus derrabus merged commit 0e3536b into doctrine:3.8.x Jun 8, 2024
85 of 86 checks passed
@derrabus derrabus added this to the 3.8.5 milestone Jun 8, 2024
@morozov
Copy link
Member

morozov commented Jun 8, 2024

What do you think about testing against MariaDB snapshots?

I think this is overkill. New versions of databases are adopted slower than say PHP versions. It is fine to start testing agains a new version once it's released and catch up by the time its adoption begins.

@grooverdan grooverdan deleted the mariadb_ci.x branch June 9, 2024 00:23
@grooverdan grooverdan mentioned this pull request Jun 9, 2024
Closed
derrabus added a commit that referenced this pull request Jun 14, 2024
@derrabus
Merge branch '4.0.x' into 4.1.x
5caaee4 
* 4.0.x:
  CI: Update MariaDB versions (#6426)
  CI MariaDB: add 11.4, remove 11.0 (#6432)
  Display warnings when running PHPUnit in CI (#6431)
  Fix typo in the portability documentation (#6430)
  Fix MariaDB fetching of default table character-set (#6361) (#6425)
  Fix the portability documentation (#6429)
  Update tests/Platforms/AbstractPlatformTestCase.php
  Update tests/Platforms/AbstractPlatformTestCase.php
  add test
  Fix: Skip type comparison if disableTypeComments is true
derrabus added a commit to derrabus/dbal that referenced this pull request Jun 18, 2024
@derrabus
Merge branch '3.8.x' into 3.9.x
123d2f3 
* 3.8.x:
  PHPStan 1.11.5 (doctrine#6446)
  Revert "Merge pull request doctrine#6413 from achterin/bugfix/foreign_key_name_change_detection"
  CI: Update MariaDB versions (doctrine#6426)
  CI MariaDB: add 11.4, remove 11.0 (doctrine#6432)
  Fix typo in the portability documentation (doctrine#6430)
derrabus added a commit to derrabus/dbal that referenced this pull request Jun 18, 2024
@derrabus
Merge branch '3.8.x' into 3.9.x
33ce3c4 
* 3.8.x:
  Move schema split for SQLite CREATE INDEX only (doctrine#6352)
  PHPStan 1.11.5 (doctrine#6446)
  Revert "Merge pull request doctrine#6413 from achterin/bugfix/foreign_key_name_change_detection"
  CI: Update MariaDB versions (doctrine#6426)
  CI MariaDB: add 11.4, remove 11.0 (doctrine#6432)
  Fix typo in the portability documentation (doctrine#6430)
derrabus added a commit to derrabus/dbal that referenced this pull request Jun 19, 2024
@derrabus
Merge branch '4.1.x' into 5.0.x
819ac6a 
* 4.1.x: (25 commits)
  Simplify signature of fetchTableOptionsByTable
  Add MariaDb1010Platform for fetchTableOptionsByTable
  PHPUnit 10.5.21 (doctrine#6447)
  Move schema split for SQLite CREATE INDEX only (doctrine#6352)
  PHPStan 1.11.5 (doctrine#6446)
  Default to distinct union queries (doctrine#6439)
  Revert "Merge pull request doctrine#6413 from achterin/bugfix/foreign_key_name_change_detection"
  Add `QueryBuilder` support for `UNION` clause (doctrine#6369)
  CI: Update MariaDB versions (doctrine#6426)
  CI MariaDB: add 11.4, remove 11.0 (doctrine#6432)
  Display warnings when running PHPUnit in CI (doctrine#6431)
  Fix typo in the portability documentation (doctrine#6430)
  Fix MariaDB fetching of default table character-set (doctrine#6361) (doctrine#6425)
  Fix the portability documentation (doctrine#6429)
  Update tests/Platforms/AbstractPlatformTestCase.php
  Update tests/Platforms/AbstractPlatformTestCase.php
  add test
  Fix: Skip type comparison if disableTypeComments is true
  Remove redundant variable (doctrine#6326)
  Fix test names to reflect their actual purpose
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@derrabus derrabus derrabus approved these changes

Assignees
No one assigned
Labels
CI MariaDB
Projects
None yet
Milestone
3.8.5
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@grooverdan @derrabus @morozov