You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by GitBox <gi...@apache.org> on 2021/05/07 13:05:59 UTC
[GitHub] [drill] luocooong commented on a change in pull request #2217: DRILL-7911 Use TestContainers-MySQL instead of wix-embedded-mysql
luocooong commented on a change in pull request #2217:
URL: https://github.com/apache/drill/pull/2217#discussion_r628186209
##########
File path: pom.xml
##########
@@ -1726,7 +1740,7 @@
<dependency>
<groupId>io.netty</groupId>
<artifactId>netty-tcnative</artifactId>
- <version>2.0.1.Final</version>
+ <version>2.0.39.Final</version>
Review comment:
Could you please explain the reasons for this upgrade?
##########
File path: pom.xml
##########
@@ -110,16 +110,18 @@
<codemodel.version>2.6</codemodel.version>
<joda.version>2.10.5</joda.version>
<javax.el.version>3.0.0</javax.el.version>
+ <jna.version>5.8.0</jna.version>
Review comment:
Could you please explain that the function of this library?
##########
File path: contrib/storage-jdbc/src/test/java/org/apache/drill/exec/store/jdbc/TestJdbcPluginWithMySQLIT.java
##########
@@ -51,51 +49,57 @@
@Category(JdbcStorageTest.class)
public class TestJdbcPluginWithMySQLIT extends ClusterTest {
- private static EmbeddedMysql mysqld;
+ private static JdbcDatabaseContainer<?> jdbcContainer;
@BeforeClass
public static void initMysql() throws Exception {
startCluster(ClusterFixture.builder(dirTestWatcher));
+ String osName = System.getProperty("os.name").toLowerCase();
String mysqlDBName = "drill_mysql_test";
- int mysqlPort = QueryTestUtil.getFreePortNumber(2215, 300);
- MysqldConfig config = MysqldConfig.aMysqldConfig(Version.v5_7_27)
- .withPort(mysqlPort)
- .withUser("mysqlUser", "mysqlPass")
- .withTimeZone(DateTimeZone.UTC.toTimeZone())
- .build();
+ DockerImageName imageName;
+ if (osName.startsWith("linux") && "aarch64".equals(System.getProperty("os.arch"))) {
+ imageName = DockerImageName.parse("mariadb:10.6.0").asCompatibleSubstituteFor("mysql");
+ } else {
+ imageName = DockerImageName.parse("mysql:5.7.27");
Review comment:
Could you please defined the version of db in the final variable?
##########
File path: pom.xml
##########
@@ -1161,6 +1165,16 @@
<!-- Managed Dependencies -->
<dependencyManagement>
<dependencies>
+ <dependency>
Review comment:
Can you add these library to the specified sub-module (Unless all the sub-moudle depend on it)?
##########
File path: pom.xml
##########
@@ -110,16 +110,18 @@
<codemodel.version>2.6</codemodel.version>
<joda.version>2.10.5</joda.version>
<javax.el.version>3.0.0</javax.el.version>
+ <jna.version>5.8.0</jna.version>
<surefire.version>3.0.0-M4</surefire.version>
<commons.compress.version>1.20</commons.compress.version>
<hikari.version>3.4.2</hikari.version>
- <netty.version>4.1.59.Final</netty.version>
+ <netty.version>4.1.63.Final</netty.version>
Review comment:
Netty is core of Drill RPC layer. So, Please separate this change into a new PR if the upgrade is necessary (for the aarch64).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org