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

upgrade mysql:mysql-connector-java to 8.2.0 #16024

Merged
merged 3 commits into from
May 6, 2024

Conversation

AlbericByte
Copy link
Contributor

Fixes #13389.

Description

upgrade the mysql:mysql-connector-java to 8.0.28

upgrade the package version

Release note

Upgrade mysql:mysql-connector-java to 8.0.28


Key changed/added classes in this PR
  • MySQLConnector.java
  • MySQLConnectorTest.java
  • pom.xml

This PR has:
  • been self-reviewed.
  • using the concurrency checklist (Remove this item if the PR doesn't have any relation to concurrency.)
  • added documentation for new or modified features or behaviors.
  • a release note entry in the PR description.
  • added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
  • added or updated version, license, or notice information in licenses.yaml
  • added comments explaining the "why" and the intent of the code wherever would not be obvious for an unfamiliar reader.
  • added unit tests or modified existing tests to cover new code paths, ensuring the threshold for code coverage is met.
  • added integration tests.
  • been tested in a test Druid cluster.

@cryptoe
Copy link
Contributor

cryptoe commented Mar 4, 2024

Similar work happening in this PR as well: https://github.com/apache/druid/pull/15408/files

@AlbericByte AlbericByte changed the title upgrade mysql:mysql-connector-java to 8.0.28 upgrade mysql:mysql-connector-java to 8.2.0 Apr 15, 2024

@Test
public void testWhenInvalidUrlFormat()
{
Copy link
Member

Choose a reason for hiding this comment

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

why is this removed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The parser no longer checks if every component is in the expected format it only checks if the URL starts with one of the required schema i.e jdbc:mysql,jdbc:mysql:loadbalancer, etc. So we will not be able to determine if this is a valid/invalid URL just by using the parser.

The way to determine if the URL is valid is by creating a connection with the DB.
😄

Assert.assertTrue(
connector.connectorIsTransientException(new SQLException("some transient failure", "s0", 1317))
);
Assert.assertFalse(
connector.connectorIsTransientException(new SQLException("totally realistic test data", "s0", 1337))
);
// this method does not specially handle normal transient exceptions either, since it is not vendor specific
Assert.assertFalse(
Copy link
Member

Choose a reason for hiding this comment

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

why is it changed from assertFalse to assertTrue

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your time first.

  • We changed MYSQL_TRANSIENT_EXCEPTION_CLASS_NAME from "com.mysql.jdbc.exceptions.MySQLTransientException" to "java.sql.SQLTransientException"

  • And in mysql-connector-j-8.2.0.jar, we do not have MySQLTransientException.

@@ -338,29 +337,6 @@ public boolean isEnforceAllowedProperties()
);
}

@Test
public void testFindPropertyKeysFromInvalidConnectUrl()
Copy link
Member

Choose a reason for hiding this comment

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

it makes sense to remove this duplicate test

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

Copy link
Member

@FrankChen021 FrankChen021 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 the contribution.

@FrankChen021 FrankChen021 merged commit 92fb0ff into apache:master May 6, 2024
88 checks passed
@AlbericByte AlbericByte deleted the mysql branch May 6, 2024 17:08
cryptoe pushed a commit that referenced this pull request Sep 3, 2024
* upgrade mysql:mysql-connector-java to 8.2.0

* fix the check errors

* remove unused comment

(cherry picked from commit 92fb0ff)
@kfaraz kfaraz added this to the 31.0.0 milestone Oct 4, 2024
@cryptoe cryptoe modified the milestones: 31.0.0, 30.0.1 Oct 9, 2024
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.

4 participants