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.

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

Update MariaDB containers #9554

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Update MariaDB containers #9554

wants to merge 1 commit into from

Conversation

rusher
Copy link

@rusher rusher commented Nov 25, 2024

  • update connector to recent version
  • set default image to current LTS (10.3 is EOL)
  • add Foundation image from quay.io: these permit testing preview mariadb images, available 6 months before official release, like new authentication, vector, ...

* update connector to recent version
* set default image to current LTS (10.3 is EOL)
* add Foundation image from quay.io: these permit testing preview mariadb images, available 6 months before official release, like new authentication, vector, ...
@rusher rusher requested a review from a team as a code owner November 25, 2024 11:31
Copy link
Member

@eddumelendez eddumelendez left a comment

Choose a reason for hiding this comment

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

Hi @rusher, thanks for your contribution. I'left some comments regarding to the changes introduced in this PR. I'm not sure about introducing the image hosted in quay.

@Deprecated
public static final String DEFAULT_TAG = "10.3.6";
public static final String DEFAULT_TAG = "10.11.10";
Copy link
Member

Choose a reason for hiding this comment

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

default constructor is deprecated and for those using it will bring a new version, which is unexpected. User must declare the version they want to use

Comment on lines +7 to +13
compileOnly 'org.mariadb:r2dbc-mariadb:1.3.0'

testImplementation project(':jdbc-test')
testImplementation 'org.mariadb.jdbc:mariadb-java-client:3.4.1'
testImplementation 'org.mariadb.jdbc:mariadb-java-client:3.5.1'

testImplementation testFixtures(project(':r2dbc'))
testRuntimeOnly 'org.mariadb:r2dbc-mariadb:1.0.3'
testRuntimeOnly 'org.mariadb:r2dbc-mariadb:1.3.0'
Copy link
Member

Choose a reason for hiding this comment

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

we use dependabot to update our dependencies. Please revert

@@ -144,6 +144,18 @@ public void testEmptyPasswordWithRootUser() throws SQLException {
}
}

@Test
public void testFondationImages() throws SQLException {
try (MariaDBContainer<?> mysql = new MariaDBContainer<>("quay.io/repository/mariadb-foundation/mariadb-devel?tab=tags&tag=11.7").withUsername("root")) {
Copy link
Member

Choose a reason for hiding this comment

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

any specific reason to declare .withUsername("root")? It's great to have a test, however, quay has had many outages in the past https://status.redhat.com/history. Not sure we want to cause those failures on our CI system.

Choose a reason for hiding this comment

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

Filtering to the quay.io registry component there seems to be one incident in August 2024 since February 2024 when they migrated there. The March 2024 incident looked like deployment rather than pulls.

Did you experience more than these?

There's currently about 300-400 pulls per day on the mariadb-devel image so I'm just hoping its reliable enough for everyone.

@@ -17,8 +17,10 @@ public class MariaDBContainer<SELF extends MariaDBContainer<SELF>> extends JdbcD

private static final DockerImageName DEFAULT_IMAGE_NAME = DockerImageName.parse("mariadb");

private static final DockerImageName FOUNDATION_IMAGE_NAME = DockerImageName.parse("quay.io/repository/mariadb-foundation/mariadb-devel");

Choose a reason for hiding this comment

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

/repository/ shouldn't be part of this. It won't resolve otherwise:

$ podman pull 'quay.io/repository/mariadb-foundation/mariadb-devel:11.7'
Trying to pull quay.io/repository/mariadb-foundation/mariadb-devel:11.7...
Error: initializing source docker://quay.io/repository/mariadb-foundation/mariadb-devel:11.7: reading manifest 11.7 in quay.io/repository/mariadb-foundation/mariadb-devel: unauthorized: access to the requested resource is not authorized

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants