You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@hive.apache.org by GitBox <gi...@apache.org> on 2022/01/10 11:59:17 UTC

[GitHub] [hive] zabetak commented on a change in pull request #2919: HIVE-25667: Unify code managing JDBC databases in tests

zabetak commented on a change in pull request #2919:
URL: https://github.com/apache/hive/pull/2919#discussion_r781110385



##########
File path: standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/AbstractDatabase.java
##########
@@ -0,0 +1,302 @@
+package org.apache.hadoop.hive.metastore.dbinstall;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.hadoop.hive.metastore.tools.schematool.MetastoreSchemaTool;
+import org.junit.rules.ExternalResource;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import sqlline.SqlLine;
+
+import java.io.BufferedReader;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.InputStreamReader;
+import java.io.PrintStream;
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.SQLException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+
+public abstract class AbstractDatabase extends ExternalResource {

Review comment:
       Do we want to keep this dependent to `ExternalResource` and junit APis in general?

##########
File path: standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/AbstractDatabase.java
##########
@@ -0,0 +1,302 @@
+package org.apache.hadoop.hive.metastore.dbinstall;

Review comment:
       Isn't `hive-testutils` module a better place for keeping these classes since they are not only used in the `metastore-server`?

##########
File path: standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/AbstractDatabase.java
##########
@@ -0,0 +1,302 @@
+package org.apache.hadoop.hive.metastore.dbinstall;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.hadoop.hive.metastore.tools.schematool.MetastoreSchemaTool;
+import org.junit.rules.ExternalResource;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import sqlline.SqlLine;
+
+import java.io.BufferedReader;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.InputStreamReader;
+import java.io.PrintStream;
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.SQLException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+
+public abstract class AbstractDatabase extends ExternalResource {
+
+  private final Logger LOG = LoggerFactory.getLogger(AbstractDatabase.class);
+
+  private final int MAX_STARTUP_WAIT = 5 * 60 * 1000;
+  private final String HIVE_USER = "hiveuser";
+  private final String HIVE_PASSWORD = "hivepassword";
+
+  private String db = "hivedb";
+  private boolean verbose = System.getProperty("verbose.schematool") != null;
+  protected boolean useDockerDatabaseArg = false;
+
+  public AbstractDatabase setVerbose(boolean verbose) {
+    this.verbose = verbose;
+    return this;
+  }
+
+  public AbstractDatabase setUseDockerDatabaseArg(boolean useDockerDatabaseArg) {
+    this.useDockerDatabaseArg = useDockerDatabaseArg;
+    return this;
+  }
+
+  public AbstractDatabase setDb(String db) {
+    this.db = db;
+    return this;
+  }
+
+  protected abstract String getDockerImageName();
+  protected abstract List<String> getDockerBaseArgs();
+  protected abstract String getDbType();
+  protected abstract String getDbRootUser();
+  protected abstract String getDbRootPassword();
+  public abstract String getJdbcDriver();
+  protected abstract String getJdbcUrl(String hostAddress);
+  protected abstract boolean isContainerReady(ProcessResults pr);
+
+  public String getJdbcUrl() {
+    return getJdbcUrl(getContainerHostAddress());
+  }
+
+  protected String getDockerDatabaseArg() {
+    return null;
+  }
+
+  /**
+   * URL to use when connecting as root rather than Hive
+   *
+   * @return URL
+   */
+  public abstract String getInitialJdbcUrl(String hostAddress);
+
+  public final String getInitialJdbcUrl() {
+    return getInitialJdbcUrl(getContainerHostAddress());
+  }
+
+  public String getHiveUser(){
+    return HIVE_USER;
+  }
+
+  public String getHivePassword(){
+    return HIVE_PASSWORD;
+  }
+
+  protected String getDb() {
+    return db;
+  }
+
+  private String getDockerContainerName() {
+    return String.format("testDb-%s", getClass().getSimpleName());
+  }
+
+  private List<String> getDockerAdditionalArgs() {
+    List<String> dockerArgs = getDockerBaseArgs();
+    if (useDockerDatabaseArg && StringUtils.isNotEmpty(getDockerDatabaseArg())) {
+      dockerArgs.addAll(Arrays.asList("-e", getDockerDatabaseArg()));
+    }
+    return dockerArgs;
+  }
+
+  private String[] buildRunCmd() {
+    List<String> cmd =  new ArrayList(
+        Arrays.asList("docker", "run", "--rm", "--name"));
+    cmd.add(getDockerContainerName());
+    cmd.addAll(getDockerAdditionalArgs());
+    cmd.add(getDockerImageName());
+    return cmd.toArray(new String[cmd.size()]);
+  }
+
+  private String getContainerHostAddress() {
+    String hostAddress = System.getenv("HIVE_TEST_DOCKER_HOST");
+    return hostAddress != null ? hostAddress : "localhost";
+  }
+
+  private String[] buildRmCmd() {
+    return new String[] { "docker", "rm", "-f", "-v", getDockerContainerName() };
+  }
+
+  private String[] buildLogCmd() {
+    return new String[] { "docker", "logs", getDockerContainerName() };
+  }
+
+  private int runCmdAndPrintStreams(String[] cmd, long secondsToWait)
+      throws InterruptedException, IOException {
+    ProcessResults results = runCmd(cmd, secondsToWait);
+    LOG.info("Stdout from proc: " + results.stdout);
+    LOG.info("Stderr from proc: " + results.stderr);
+    return results.rc;
+  }
+
+  private ProcessResults runCmd(String[] cmd, long secondsToWait)
+      throws IOException, InterruptedException {
+    LOG.info("Going to run: " + StringUtils.join(cmd, " "));
+    Process proc = Runtime.getRuntime().exec(cmd);
+    if (!proc.waitFor(secondsToWait, TimeUnit.SECONDS)) {
+      throw new RuntimeException(
+          "Process " + cmd[0] + " failed to run in " + secondsToWait + " seconds");
+    }
+    BufferedReader reader = new BufferedReader(new InputStreamReader(proc.getInputStream()));
+    final StringBuilder lines = new StringBuilder();
+    reader.lines().forEach(s -> lines.append(s).append('\n'));
+
+    reader = new BufferedReader(new InputStreamReader(proc.getErrorStream()));
+    final StringBuilder errLines = new StringBuilder();
+    reader.lines().forEach(s -> errLines.append(s).append('\n'));
+    LOG.info("Result size: " + lines.length() + ";" + errLines.length());
+    return new ProcessResults(lines.toString(), errLines.toString(), proc.exitValue());
+  }
+
+  public void launchDockerContainer() throws Exception {
+    runCmdAndPrintStreams(buildRmCmd(), 600);
+    if (runCmdAndPrintStreams(buildRunCmd(), 600) != 0) {
+      throw new RuntimeException("Unable to start docker container");
+    }
+    long startTime = System.currentTimeMillis();
+    ProcessResults pr;
+    do {
+      Thread.sleep(1000);
+      pr = runCmd(buildLogCmd(), 30);
+      if (pr.rc != 0) {
+        throw new RuntimeException("Failed to get docker logs");
+      }
+    } while (startTime + MAX_STARTUP_WAIT >= System.currentTimeMillis() && !isContainerReady(pr));
+    if (startTime + MAX_STARTUP_WAIT < System.currentTimeMillis()) {
+      throw new RuntimeException("Container failed to be ready in " + MAX_STARTUP_WAIT/1000 +
+          " seconds");
+    }
+  }
+
+  public void cleanupDockerContainer() throws IOException, InterruptedException {
+    if (runCmdAndPrintStreams(buildRmCmd(), 600) != 0) {
+      throw new RuntimeException("Unable to remove docker container");
+    }
+  }
+
+  @Override
+  public void before() throws Exception {
+    launchDockerContainer();
+    MetastoreSchemaTool.setHomeDirForTesting();
+  }
+
+  @Override
+  public void after() {
+    if ("true".equalsIgnoreCase(System.getProperty("metastore.itest.no.stop.container"))) {
+      LOG.warn("Not stopping container " + getDockerContainerName() + " at user request, please "
+          + "be sure to shut it down before rerunning the test.");
+      return;
+    }
+    try {
+      cleanupDockerContainer();
+    } catch (InterruptedException | IOException e) {
+      e.printStackTrace();
+    }
+  }
+
+  public void install() {
+    createUser();
+    installLatest();
+  }
+
+  public int createUser() {
+    return new MetastoreSchemaTool().setVerbose(verbose).run(new String[] {
+        "-createUser",
+        "-dbType", getDbType(),
+        "-userName", getDbRootUser(),
+        "-passWord", getDbRootPassword(),
+        "-hiveUser", getHiveUser(),
+        "-hivePassword", getHivePassword(),
+        "-hiveDb", getDb(),
+        "-url", getInitialJdbcUrl(),
+        "-driver", getJdbcDriver()
+    });
+  }
+
+  public int installLatest() {
+    return new MetastoreSchemaTool().setVerbose(verbose).run(new String[] {
+        "-initSchema",
+        "-dbType", getDbType(),
+        "-userName", getHiveUser(),
+        "-passWord", getHivePassword(),
+        "-url", getJdbcUrl(),
+        "-driver", getJdbcDriver(),
+        "-verbose"
+    });
+  }
+
+  public int installAVersion(String version) {
+    return new MetastoreSchemaTool().setVerbose(verbose).run(new String[] {
+        "-initSchemaTo", version,
+        "-dbType", getDbType(),
+        "-userName", getHiveUser(),
+        "-passWord", getHivePassword(),
+        "-url", getJdbcUrl(),
+        "-driver", getJdbcDriver()
+    });
+  }
+
+  public int upgradeToLatest() {
+    return new MetastoreSchemaTool().setVerbose(verbose).run(new String[] {
+        "-upgradeSchema",
+        "-dbType", getDbType(),
+        "-userName", getHiveUser(),
+        "-passWord", getHivePassword(),
+        "-url", getJdbcUrl(),
+        "-driver", getJdbcDriver()
+    });
+  }
+
+  public int validateSchema() {
+    return new MetastoreSchemaTool().setVerbose(verbose).run(new String[] {
+        "-validate",
+        "-dbType", getDbType(),
+        "-userName", getHiveUser(),
+        "-passWord", getHivePassword(),
+        "-url", getJdbcUrl(),
+        "-driver", getJdbcDriver()
+    });
+  }

Review comment:
       Possibly we could do this in separate refactoring/jira but these methods are not necessary for all consumers of this API so we could put them in a separate class/interface. 

##########
File path: standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/rules/Mssql.java
##########
@@ -63,7 +64,8 @@ public String getJdbcDriver() {
 
   @Override
   public String getJdbcUrl(String hostAddress) {
-    return "jdbc:sqlserver://" + hostAddress + ":1433;DatabaseName=" + HIVE_DB + ";";
+    //Mssql doesn't support database parameter in docker

Review comment:
       I don't understand what the comment means. Can you elaborate more?

##########
File path: standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/rules/Mariadb.java
##########
@@ -18,19 +18,31 @@
 
 package org.apache.hadoop.hive.metastore.dbinstall.rules;
 
+import org.apache.hadoop.hive.metastore.dbinstall.AbstractDatabase;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
-public class Mariadb extends DatabaseRule {
+public class Mariadb extends AbstractDatabase {
 
   @Override
   public String getDockerImageName() {
     return "mariadb:10.2";
   }
 
   @Override
-  public String[] getDockerAdditionalArgs() {
-    return buildArray("-p", "3306:3306", "-e", "MYSQL_ROOT_PASSWORD=" + getDbRootPassword(), "-d");
+  public String getDockerDatabaseArg() {
+    return "MARIADB_DATABASE=" + getDb();
+  }
+
+  @Override
+  public List<String> getDockerBaseArgs() {
+    return new ArrayList(Arrays.asList("-p", "3309:3306",

Review comment:
       Why are we wrapping into ArrayList? Are we going to modify the list? Keeping things immutable makes the code easier to understand and use.

##########
File path: standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/AbstractDatabase.java
##########
@@ -0,0 +1,302 @@
+package org.apache.hadoop.hive.metastore.dbinstall;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.hadoop.hive.metastore.tools.schematool.MetastoreSchemaTool;
+import org.junit.rules.ExternalResource;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import sqlline.SqlLine;
+
+import java.io.BufferedReader;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.InputStreamReader;
+import java.io.PrintStream;
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.SQLException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+
+public abstract class AbstractDatabase extends ExternalResource {
+
+  private final Logger LOG = LoggerFactory.getLogger(AbstractDatabase.class);
+
+  private final int MAX_STARTUP_WAIT = 5 * 60 * 1000;
+  private final String HIVE_USER = "hiveuser";
+  private final String HIVE_PASSWORD = "hivepassword";
+
+  private String db = "hivedb";
+  private boolean verbose = System.getProperty("verbose.schematool") != null;
+  protected boolean useDockerDatabaseArg = false;
+
+  public AbstractDatabase setVerbose(boolean verbose) {
+    this.verbose = verbose;
+    return this;
+  }
+
+  public AbstractDatabase setUseDockerDatabaseArg(boolean useDockerDatabaseArg) {
+    this.useDockerDatabaseArg = useDockerDatabaseArg;
+    return this;
+  }
+
+  public AbstractDatabase setDb(String db) {
+    this.db = db;
+    return this;
+  }
+
+  protected abstract String getDockerImageName();
+  protected abstract List<String> getDockerBaseArgs();
+  protected abstract String getDbType();
+  protected abstract String getDbRootUser();
+  protected abstract String getDbRootPassword();
+  public abstract String getJdbcDriver();
+  protected abstract String getJdbcUrl(String hostAddress);
+  protected abstract boolean isContainerReady(ProcessResults pr);
+
+  public String getJdbcUrl() {
+    return getJdbcUrl(getContainerHostAddress());
+  }
+
+  protected String getDockerDatabaseArg() {
+    return null;
+  }
+
+  /**
+   * URL to use when connecting as root rather than Hive
+   *
+   * @return URL
+   */
+  public abstract String getInitialJdbcUrl(String hostAddress);
+
+  public final String getInitialJdbcUrl() {
+    return getInitialJdbcUrl(getContainerHostAddress());
+  }
+
+  public String getHiveUser(){
+    return HIVE_USER;
+  }
+
+  public String getHivePassword(){
+    return HIVE_PASSWORD;
+  }
+
+  protected String getDb() {
+    return db;
+  }
+
+  private String getDockerContainerName() {
+    return String.format("testDb-%s", getClass().getSimpleName());
+  }
+
+  private List<String> getDockerAdditionalArgs() {
+    List<String> dockerArgs = getDockerBaseArgs();
+    if (useDockerDatabaseArg && StringUtils.isNotEmpty(getDockerDatabaseArg())) {
+      dockerArgs.addAll(Arrays.asList("-e", getDockerDatabaseArg()));
+    }
+    return dockerArgs;
+  }
+
+  private String[] buildRunCmd() {
+    List<String> cmd =  new ArrayList(
+        Arrays.asList("docker", "run", "--rm", "--name"));
+    cmd.add(getDockerContainerName());
+    cmd.addAll(getDockerAdditionalArgs());
+    cmd.add(getDockerImageName());
+    return cmd.toArray(new String[cmd.size()]);
+  }
+
+  private String getContainerHostAddress() {
+    String hostAddress = System.getenv("HIVE_TEST_DOCKER_HOST");
+    return hostAddress != null ? hostAddress : "localhost";
+  }
+
+  private String[] buildRmCmd() {
+    return new String[] { "docker", "rm", "-f", "-v", getDockerContainerName() };
+  }
+
+  private String[] buildLogCmd() {
+    return new String[] { "docker", "logs", getDockerContainerName() };
+  }
+
+  private int runCmdAndPrintStreams(String[] cmd, long secondsToWait)
+      throws InterruptedException, IOException {
+    ProcessResults results = runCmd(cmd, secondsToWait);
+    LOG.info("Stdout from proc: " + results.stdout);
+    LOG.info("Stderr from proc: " + results.stderr);
+    return results.rc;
+  }
+
+  private ProcessResults runCmd(String[] cmd, long secondsToWait)
+      throws IOException, InterruptedException {
+    LOG.info("Going to run: " + StringUtils.join(cmd, " "));
+    Process proc = Runtime.getRuntime().exec(cmd);
+    if (!proc.waitFor(secondsToWait, TimeUnit.SECONDS)) {
+      throw new RuntimeException(
+          "Process " + cmd[0] + " failed to run in " + secondsToWait + " seconds");
+    }
+    BufferedReader reader = new BufferedReader(new InputStreamReader(proc.getInputStream()));
+    final StringBuilder lines = new StringBuilder();
+    reader.lines().forEach(s -> lines.append(s).append('\n'));
+
+    reader = new BufferedReader(new InputStreamReader(proc.getErrorStream()));
+    final StringBuilder errLines = new StringBuilder();
+    reader.lines().forEach(s -> errLines.append(s).append('\n'));
+    LOG.info("Result size: " + lines.length() + ";" + errLines.length());
+    return new ProcessResults(lines.toString(), errLines.toString(), proc.exitValue());
+  }
+
+  public void launchDockerContainer() throws Exception {
+    runCmdAndPrintStreams(buildRmCmd(), 600);
+    if (runCmdAndPrintStreams(buildRunCmd(), 600) != 0) {
+      throw new RuntimeException("Unable to start docker container");
+    }
+    long startTime = System.currentTimeMillis();
+    ProcessResults pr;
+    do {
+      Thread.sleep(1000);
+      pr = runCmd(buildLogCmd(), 30);
+      if (pr.rc != 0) {
+        throw new RuntimeException("Failed to get docker logs");
+      }
+    } while (startTime + MAX_STARTUP_WAIT >= System.currentTimeMillis() && !isContainerReady(pr));
+    if (startTime + MAX_STARTUP_WAIT < System.currentTimeMillis()) {
+      throw new RuntimeException("Container failed to be ready in " + MAX_STARTUP_WAIT/1000 +
+          " seconds");
+    }
+  }
+
+  public void cleanupDockerContainer() throws IOException, InterruptedException {
+    if (runCmdAndPrintStreams(buildRmCmd(), 600) != 0) {
+      throw new RuntimeException("Unable to remove docker container");
+    }
+  }
+
+  @Override
+  public void before() throws Exception {
+    launchDockerContainer();
+    MetastoreSchemaTool.setHomeDirForTesting();
+  }
+
+  @Override
+  public void after() {
+    if ("true".equalsIgnoreCase(System.getProperty("metastore.itest.no.stop.container"))) {
+      LOG.warn("Not stopping container " + getDockerContainerName() + " at user request, please "
+          + "be sure to shut it down before rerunning the test.");
+      return;
+    }
+    try {
+      cleanupDockerContainer();
+    } catch (InterruptedException | IOException e) {
+      e.printStackTrace();
+    }
+  }

Review comment:
       Possibly these methods can go to those classes using this class as a rule. Another alternative would be to create a rule class which wraps an instance of `AbstractDatabase` and provides these methods.

##########
File path: standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/rules/Derby.java
##########
@@ -42,7 +46,7 @@ public String getDockerImageName() {
   }
 
   @Override
-  public String[] getDockerAdditionalArgs() {
+  public List<String> getDockerBaseArgs() {
     return null;

Review comment:
       Effective Java Item 43: Return empty arrays or collections, not nulls

##########
File path: standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/dbinstall/AbstractDatabase.java
##########
@@ -0,0 +1,302 @@
+package org.apache.hadoop.hive.metastore.dbinstall;
+
+import org.apache.commons.lang3.StringUtils;
+import org.apache.hadoop.hive.metastore.tools.schematool.MetastoreSchemaTool;
+import org.junit.rules.ExternalResource;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import sqlline.SqlLine;
+
+import java.io.BufferedReader;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.InputStreamReader;
+import java.io.PrintStream;
+import java.sql.Connection;
+import java.sql.DriverManager;
+import java.sql.SQLException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.concurrent.TimeUnit;
+
+public abstract class AbstractDatabase extends ExternalResource {
+
+  private final Logger LOG = LoggerFactory.getLogger(AbstractDatabase.class);
+
+  private final int MAX_STARTUP_WAIT = 5 * 60 * 1000;
+  private final String HIVE_USER = "hiveuser";
+  private final String HIVE_PASSWORD = "hivepassword";
+
+  private String db = "hivedb";
+  private boolean verbose = System.getProperty("verbose.schematool") != null;
+  protected boolean useDockerDatabaseArg = false;
+
+  public AbstractDatabase setVerbose(boolean verbose) {
+    this.verbose = verbose;
+    return this;
+  }
+
+  public AbstractDatabase setUseDockerDatabaseArg(boolean useDockerDatabaseArg) {
+    this.useDockerDatabaseArg = useDockerDatabaseArg;
+    return this;
+  }
+
+  public AbstractDatabase setDb(String db) {
+    this.db = db;
+    return this;
+  }
+
+  protected abstract String getDockerImageName();
+  protected abstract List<String> getDockerBaseArgs();
+  protected abstract String getDbType();
+  protected abstract String getDbRootUser();
+  protected abstract String getDbRootPassword();
+  public abstract String getJdbcDriver();
+  protected abstract String getJdbcUrl(String hostAddress);
+  protected abstract boolean isContainerReady(ProcessResults pr);
+
+  public String getJdbcUrl() {
+    return getJdbcUrl(getContainerHostAddress());
+  }
+
+  protected String getDockerDatabaseArg() {
+    return null;
+  }
+
+  /**
+   * URL to use when connecting as root rather than Hive
+   *
+   * @return URL
+   */
+  public abstract String getInitialJdbcUrl(String hostAddress);
+
+  public final String getInitialJdbcUrl() {
+    return getInitialJdbcUrl(getContainerHostAddress());
+  }
+
+  public String getHiveUser(){
+    return HIVE_USER;
+  }
+
+  public String getHivePassword(){
+    return HIVE_PASSWORD;
+  }
+
+  protected String getDb() {
+    return db;
+  }
+
+  private String getDockerContainerName() {
+    return String.format("testDb-%s", getClass().getSimpleName());
+  }
+
+  private List<String> getDockerAdditionalArgs() {
+    List<String> dockerArgs = getDockerBaseArgs();
+    if (useDockerDatabaseArg && StringUtils.isNotEmpty(getDockerDatabaseArg())) {
+      dockerArgs.addAll(Arrays.asList("-e", getDockerDatabaseArg()));
+    }
+    return dockerArgs;
+  }

Review comment:
       Having many `getDocker*Args` method makes the code harder to follow. Does it bring a big benefit having  `getDockerBaseArgs and `getDockerDatabaseArg` separate?




-- 
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.

To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@hive.apache.org
For additional commands, e-mail: gitbox-help@hive.apache.org