-
Notifications
You must be signed in to change notification settings - Fork 3.8k
upgrade mysql:mysql-connector-java to 8.2.0 #16024
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,7 +25,6 @@ | |
| import com.google.common.collect.ImmutableSet; | ||
| import nl.jqno.equalsverifier.EqualsVerifier; | ||
| import org.apache.druid.jackson.DefaultObjectMapper; | ||
| import org.apache.druid.java.util.common.StringUtils; | ||
| import org.apache.druid.metadata.MetadataStorageConnectorConfig; | ||
| import org.apache.druid.metadata.storage.mysql.MySQLConnectorDriverConfig; | ||
| import org.apache.druid.metadata.storage.mysql.MySQLMetadataStorageModule; | ||
|
|
@@ -338,29 +337,6 @@ public boolean isEnforceAllowedProperties() | |
| ); | ||
| } | ||
|
|
||
| @Test | ||
| public void testFindPropertyKeysFromInvalidConnectUrl() | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it makes sense to remove this duplicate test
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
| { | ||
| final String url = "jdbc:mysql:/invalid-url::3006"; | ||
| MetadataStorageConnectorConfig connectorConfig = new MetadataStorageConnectorConfig() | ||
| { | ||
| @Override | ||
| public String getConnectURI() | ||
| { | ||
| return url; | ||
| } | ||
| }; | ||
|
|
||
| expectedException.expect(RuntimeException.class); | ||
| expectedException.expectMessage(StringUtils.format("Invalid URL format for MySQL: [%s]", url)); | ||
| new MySQLFirehoseDatabaseConnector( | ||
| connectorConfig, | ||
| null, | ||
| new JdbcAccessSecurityConfig(), | ||
| mySQLConnectorDriverConfig | ||
| ); | ||
| } | ||
|
|
||
| private static JdbcAccessSecurityConfig newSecurityConfigEnforcingAllowList(Set<String> allowedProperties) | ||
| { | ||
| return new JdbcAccessSecurityConfig() | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,8 +20,6 @@ | |
| package org.apache.druid.metadata.storage.mysql; | ||
|
|
||
| import com.google.common.base.Supplier; | ||
| import com.mysql.jdbc.exceptions.MySQLTransactionRollbackException; | ||
| import com.mysql.jdbc.exceptions.MySQLTransientException; | ||
| import org.apache.druid.metadata.MetadataStorageConnectorConfig; | ||
| import org.apache.druid.metadata.MetadataStorageTablesConfig; | ||
| import org.junit.Assert; | ||
|
|
@@ -57,16 +55,13 @@ public void testIsExceptionTransientMySql() | |
| new MySQLConnectorSslConfig(), | ||
| MYSQL_DRIVER_CONFIG | ||
| ); | ||
| Assert.assertTrue(connector.connectorIsTransientException(new MySQLTransientException())); | ||
| Assert.assertTrue(connector.connectorIsTransientException(new MySQLTransactionRollbackException())); | ||
| 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( | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why is it changed from assertFalse to assertTrue
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for your time first.
|
||
| Assert.assertTrue( | ||
| connector.connectorIsTransientException(new SQLTransientConnectionException("transient")) | ||
| ); | ||
| } | ||
|
|
@@ -81,7 +76,7 @@ public void testIsExceptionTransientNoMySqlClazz() | |
| MARIADB_DRIVER_CONFIG | ||
| ); | ||
| // no vendor specific for MariaDb, so should always be false | ||
| Assert.assertFalse(connector.connectorIsTransientException(new MySQLTransientException())); | ||
| Assert.assertFalse(connector.connectorIsTransientException(new SQLTransientException())); | ||
| Assert.assertFalse( | ||
| connector.connectorIsTransientException(new SQLException("some transient failure", "s0", 1317)) | ||
| ); | ||
|
|
@@ -116,7 +111,7 @@ public void testIsRootCausePacketTooBigException() | |
| connector.isRootCausePacketTooBigException(new SQLTransientException()) | ||
| ); | ||
| Assert.assertFalse( | ||
| connector.isRootCausePacketTooBigException(new MySQLTransientException()) | ||
| connector.isRootCausePacketTooBigException(new SQLTransientException()) | ||
| ); | ||
| } | ||
|
|
||
|
|
||
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.
why is this removed
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.
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.
😄