Skip to content

Support MySQL 8#1470

Merged
rnorth merged 9 commits into
masterfrom
mysql-8-compatibility
May 27, 2019
Merged

Support MySQL 8#1470
rnorth merged 9 commits into
masterfrom
mysql-8-compatibility

Conversation

@rnorth
Copy link
Copy Markdown
Member

@rnorth rnorth commented May 15, 2019

Fixes #736
Replaces #1168

Note that this PR includes some long overdue refactoring of the database container tests - it made sense to do this as I was adding another test class, and doing so without refactoring would have worsened duplication.

The actual substance of the change is quite small, and visible in the MySQLContainer class.

Quoting the rationale:

Testcontainers has the driver as a runtime dependency, not transitive, so it's completely up to the user to choose which driver they need.

I've just tried this out, modifying our jdbc tests to use the newer driver, and found a few things:

The v8 driver seems to be backwards compatible (as you'd hope!) with 5.5, 5.6, 5.7.
With the v8 driver, MySqlContainer still chokes when connecting to mysql:8, throwing SQLNonTransientConnectionException: Public Key Retrieval is not allowed

However, this can be fixed by adding allowPublicKeyRetrieval=true to the query string params, as we already do with useSSL=false.

The allowPublicKeyRetrieval option goes back as far as MySql 5.1, (which predates Docker Hub) and seems to be compatible with any pre-8 image that we'd want to test.

Thus, I now have the following test code working without problems:

for (String tag : asList("5.5", "5.6", "5.7", "8")) {
    try (final MySQLContainer container = new MySQLContainer<>("mysql:" + tag)) {
         container.start();
    }
}

rs.next();
String resultSetInt = rs.getString(2);
assertEquals("Passing query parameters to set DB connection encoding is successful", "utf8", resultSetInt);
assertEquals("Passing query parameters to set DB connection encoding is successful", "utf8mb4", resultSetInt);
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is to match the changes mentioned here

For Connector/J 8.0.13 and later:

When UTF-8 is used for characterEncoding in the connection string, it maps to the MySQL character set name utf8mb4.

url = url + separator + "useSSL=false";
}

if (! url.contains("allowPublicKeyRetrieval=")) {
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is the main substance of this change.

Copy link
Copy Markdown
Member

@kiview kiview left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for extracting AbstractContainerDatabaseTest class also.

@rnorth
Copy link
Copy Markdown
Member Author

rnorth commented May 27, 2019

Thanks @kiview - will merge!

@rnorth rnorth changed the title MySQL 8 compatibility Support MySQL 8 May 27, 2019
@rnorth rnorth merged commit 581b80b into master May 27, 2019
@delete-merged-branch delete-merged-branch Bot deleted the mysql-8-compatibility branch May 27, 2019 19:55
@rnorth rnorth added this to the next milestone May 27, 2019
@rui-ferreira
Copy link
Copy Markdown

When will you publish a release with this?

@rnorth
Copy link
Copy Markdown
Member Author

rnorth commented Jul 7, 2019

This was released in 1.11.4 🎉

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.

MySQLContainer unable to run MySQL 8

3 participants