Incorporate review comments

- Use ImmutableList instead of Arrays.List - Remove declarations for VerifyException - Minor formatting changes
prestodb · Aug 26, 2020 · 4946a61 · 4946a61
1 parent 78d46f1
commit 4946a61
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 36 deletions.
diff --git a/testing-mysql-server-5/src/main/java/com/facebook/presto/testing/mysql/EmbeddedMySql5.java b/testing-mysql-server-5/src/main/java/com/facebook/presto/testing/mysql/EmbeddedMySql5.java
@@ -13,11 +13,9 @@
  */
 package com.facebook.presto.testing.mysql;
 
-import com.google.common.base.VerifyException;
 import com.google.common.collect.ImmutableList;
 
 import java.io.IOException;
-import java.util.Arrays;
 import java.util.List;
 
 final class EmbeddedMySql5
@@ -32,48 +30,54 @@ public EmbeddedMySql5(MySqlOptions mySqlOptions)
     @Override
     public List<String> getInitializationArguments()
     {
-        List<String> list = Arrays.asList(
-                "--no-defaults",
-                "--skip-sync-frm",
-                "--innodb-flush-method=nosync",
-                "--datadir=" + getDataDirectory());
+        ImmutableList.Builder<String> iList = ImmutableList.<String>builder()
+                .add(
+                    "--no-defaults",
+                    "--skip-sync-frm",
+                    "--innodb-flush-method=nosync",
+                    "--datadir=" + getDataDirectory());
 
         if (isMariadb) {
-            return ImmutableList.<String>builder().addAll(list).add("--basedir=" + getBaseDirectory()).build();
+            return iList.add("--basedir=" + getBaseDirectory()).build();
         }
         else {
-            return ImmutableList.<String>builder().addAll(list).add("--initialize-insecure").build();
+            return iList.add("--initialize-insecure").build();
         }
     }
 
     @Override
-    public List<String> getStartArguments() throws VerifyException
+    public List<String> getStartArguments()
     {
-        List<String> list = Arrays.asList(
-                "--no-defaults",
-                "--default-time-zone=+00:00",
-                "--skip-sync-frm",
-                "--innodb-flush-method=nosync",
-                "--innodb-flush-log-at-trx-commit=0",
-                "--innodb-doublewrite=0",
-                "--bind-address=localhost",
-                "--port=" + String.valueOf(getPort()),
-                "--datadir=" + getDataDirectory(),
-                "--socket=" + getSocketDirectory());
+        ImmutableList.Builder<String> iList = ImmutableList.<String>builder()
+                .add(
+                    "--no-defaults",
+                    "--default-time-zone=+00:00",
+                    "--skip-sync-frm",
+                    "--innodb-flush-method=nosync",
+                    "--innodb-flush-log-at-trx-commit=0",
+                    "--innodb-doublewrite=0",
+                    "--bind-address=localhost",
+                    "--port=" + String.valueOf(getPort()),
+                    "--datadir=" + getDataDirectory(),
+                    "--socket=" + getSocketDirectory());
 
         if (isMariadb) {
-            return ImmutableList.<String>builder().addAll(list).add(
-                "--basedir=" + getBaseDirectory(),
-                "--plugin-dir=" + getMariadbPluginDirectory(),
-                "--log-error=" + getDataDirectory() + "mariadb.log",
-                "--pid-file=" + getDataDirectory() + "mariadb.pid").build();
+            return iList
+                .add(
+                    "--basedir=" + getBaseDirectory(),
+                    "--plugin-dir=" + getMariadbPluginDirectory(),
+                    "--log-error=" + getDataDirectory() + "mariadb.log",
+                    "--pid-file=" + getDataDirectory() + "mariadb.pid")
+                .build();
         }
         else {
-            return ImmutableList.<String>builder().addAll(list).add(
-                "--skip-ssl",
-                "--disable-partition-engine-check",
-                "--explicit_defaults_for_timestamp",
-                "--lc_messages_dir=" + getShareDirectory()).build();
+            return iList
+                .add(
+                    "--skip-ssl",
+                    "--disable-partition-engine-check",
+                    "--explicit_defaults_for_timestamp",
+                    "--lc_messages_dir=" + getShareDirectory())
+                .build();
         }
     }
 }
diff --git a/...ql-server-base/src/main/java/com/facebook/presto/testing/mysql/AbstractEmbeddedMySql.java b/...ql-server-base/src/main/java/com/facebook/presto/testing/mysql/AbstractEmbeddedMySql.java
@@ -73,7 +73,7 @@ public abstract class AbstractEmbeddedMySql
     protected final boolean isMariadb = System.getProperty("os.arch").equals("ppc64le");
 
     public AbstractEmbeddedMySql(MySqlOptions mySqlOptions)
-            throws IOException, VerifyException
+            throws IOException
     {
         this.startupWait = requireNonNull(mySqlOptions.getStartupWait(), "startupWait is null");
         this.shutdownWait = requireNonNull(mySqlOptions.getShutdownWait(), "shutdownWait is null");
@@ -114,7 +114,7 @@ public Connection getMySqlDatabase()
         return DriverManager.getConnection(getJdbcUrl("root", "mysql"));
     }
 
-    protected String getMariadbInstallDb() throws VerifyException
+    protected String getMariadbInstallDb()
     {
         if (!isMariadb) {
             throw new VerifyException("mysql_install_db not applicable to non-mariadb installations");
@@ -147,7 +147,7 @@ protected String getSocketDirectory()
         return (isMariadb ? getDataDirectory() + "mysql.sock" : serverDirectory.resolve("mysql.sock").toString());
     }
 
-    protected String getMariadbPluginDirectory() throws VerifyException
+    protected String getMariadbPluginDirectory()
     {
         if (!isMariadb) {
             throw new VerifyException("--plugin-dir option not applicable to non-mariadb installations");
@@ -206,7 +206,7 @@ private static int randomPort()
         }
     }
 
-    private void initialize() throws VerifyException
+    private void initialize()
     {
         if (isMariadb) {
             system(ImmutableList.<String>builder()
@@ -223,7 +223,7 @@ private void initialize() throws VerifyException
     }
 
     private Process startMysqld()
-            throws IOException, VerifyException
+            throws IOException
     {
         Process process = new ProcessBuilder(ImmutableList.<String>builder().add(getMysqld()).addAll(getStartArguments()).build())
                 .redirectErrorStream(true)