-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Correct testcontainers for use with latest MySQL 8.3 version #8131
Conversation
Thanks for your contribution, @chadlwilson! I fixed the format and submit changes to your branch. However, test in MySQLJDBCDriverTest with URL |
Oh that's weird, I thought I'd run all the relevant tests locally and they were passing (although they are mysteriously slow with my Colima setup so maybe one got stuck and I didn't notice) I'll take a look, sorry. |
04979dc
to
faefdd6
Compare
faefdd6
to
00fcfe7
Compare
Thanks @eddumelendez - rebased with your changes and fixed the failing test by switching to a new setting to validate config overrides. Should be OK now. |
00fcfe7
to
b4336df
Compare
- 5.6 went EOL in Feb 2021. Stop testing against it. - 5.7 went EOL in Oct 2023. Retain tests for a bit, but stop using it as the default. - 8.0.34+ is now LTS rather than rolling - use this as the default for most tests - Add 8.3.0 as a candidate "innovation" version which will eventually become LTS Updates various docs to use 8.0 in the examples rather than EOL 5.7.
b4336df
to
b936ec0
Compare
Sigh; MariaDB test which shares the same test assertions should also be fixed now, although for some reason locally the MariaDB containers started by the tests just exit immediately and hang the tests so I can't verify. (Edit: This is because the images the MariaDB tests are using are also ancient, not built for arm64, and crash under QEMU emulation on Apple Silicon. MariaDB really needs similar image refreshes, but will leave it out of the scope of this to fully update these, and just do the bare minimum to ensure they run on modern Macbooks.) |
Previous images (10.2.x and 10.3.6) don't have ARM builds and otherwise crash under emulation on Apple Silicon machines. Updating accordingly is low risk even though a bigger refresh is needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @chadlwilson! Just a few questions related to the default images used in MySQLContainer and MariaDBContainer.
@@ -20,7 +20,7 @@ public class MySQLContainer<SELF extends MySQLContainer<SELF>> extends JdbcDatab | |||
private static final DockerImageName DEFAULT_IMAGE_NAME = DockerImageName.parse("mysql"); | |||
|
|||
@Deprecated | |||
public static final String DEFAULT_TAG = "5.7.34"; | |||
public static final String DEFAULT_TAG = "5.7.44"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any issue is we keep 5.7.34 as a default? I would like to avoid breaking existing test that relies on default constructor.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see why a patch/bugfix release would be expected to break things for people and if you check the history it was previously changed to a new patch releases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For what it's worth here's an example:
3aa5fbf#diff-2568f90aa8a2109ff614414c5ef2b547f6bd7e8b8d4db9e81130d091b3e5d67cR19
@@ -18,7 +18,7 @@ public class MariaDBContainer<SELF extends MariaDBContainer<SELF>> extends JdbcD | |||
private static final DockerImageName DEFAULT_IMAGE_NAME = DockerImageName.parse("mariadb"); | |||
|
|||
@Deprecated | |||
public static final String DEFAULT_TAG = "10.3.6"; | |||
public static final String DEFAULT_TAG = "10.3.39"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any issue is we keep 10.3.6 as a default? I would like to avoid breaking existing test that relies on default constructor.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes in fact, because 10.3.6 does not have ARM image variants and crashes when you try to run on Colima under emulation as noted in #8131 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
10.3.39 with multiple archs: https://hub.docker.com/layers/library/mariadb/10.3.39/images/sha256-a4b57b620852e26f2c306c79fd8a61e22c7cd6e55d0a55d5521854ed2c370102?context=explore
10.3.6 with only amd64
Thanks for your contribution, @chadlwilson ! This is merged in |
Testcontainers default configuration no longer works with MySQL
8.3.0
since they have removed theskip-host-cache
option which was deprecated in8.0.30
. This change replaces withhost_cache_size = 0
which is equivalent and backward compatible back through previous releases (incl5.7
and even5.6
).This likely breaks for people that were depending on the rolling
mysql:8
tag, since there is now a different release and LTS strategy which probably makes them more inclined to add breaking changes.8.1
(Jun 2023)8.2
(Oct 2023) and8.3
(Dec 2023) have come in rapid succession as "innovation" releases with the new release strategy. Most users should probably be testing againstmysql:8.0
tags so they stick to LTS, but that's probably a different concern :-)To avoid regressions and refresh things, this PR updates the docs and mysql testing images accordingly based on https://endoflife.date/mysql
5.6
went EOL in Feb 2021. Stop testing against it, and remove references.5.7
went EOL in Oct 2023. Retain tests for a bit, but stop using it as the default.8.0.34
+ is now LTS rather than rolling - use this as the default for most tests8.3.0
as a candidate "innovation" version to sanity test against. This could be periodically updated to stay ahead of the curve before the next LTS.