----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/25245/#review52618 -----------------------------------------------------------
jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java <https://reviews.apache.org/r/25245/#comment91476> It will be simpler and less code if we assume that we support one or more hostnames in the URL. First we extract the one or more hostnames - ie, what extractZooKeeperEnsemble is doing. Then store the hostname or list of hostnames in a variable, and replace the host in uri with a dummy hostname that is acceptable for URI parsing. After that we can extract variable names without additonal logic. You can then check if multiple hostnames are expected based on the value of SERVICE_DISCOVERY_MODE jdbc/src/java/org/apache/hive/jdbc/JdbcUriParseException.java <https://reviews.apache.org/r/25245/#comment91468> how about extending SQLException, so that you don't have to wrap it elsewhere ? jdbc/src/java/org/apache/hive/jdbc/Utils.java <https://reviews.apache.org/r/25245/#comment91471> Lets use jiras for tracking planed changes instead of TODOs in code. jdbc/src/java/org/apache/hive/jdbc/Utils.java <https://reviews.apache.org/r/25245/#comment91470> I don't think we need these TODOs here. We are already tracking them in Jiras. jdbc/src/java/org/apache/hive/jdbc/Utils.java <https://reviews.apache.org/r/25245/#comment91473> the old variable name jdbcURI is reasonable/good, as URI is an acryonym. Keeping the old name will avoid unnecessary diffs. jdbc/src/java/org/apache/hive/jdbc/Utils.java <https://reviews.apache.org/r/25245/#comment91474> You can just just SERVICE_DISCOVERY_MODE_ZOOKEEPER instead of JdbcConnectionParams.SERVICE_DISCOVERY_MODE_ZOOKEEPER - Thejas Nair On Sept. 8, 2014, 7:43 a.m., Vaibhav Gumashta wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/25245/ > ----------------------------------------------------------- > > (Updated Sept. 8, 2014, 7:43 a.m.) > > > Review request for hive, Alan Gates, Navis Ryu, Szehon Ho, and Thejas Nair. > > > Bugs: HIVE-7935 > https://issues.apache.org/jira/browse/HIVE-7935 > > > Repository: hive-git > > > Description > ------- > > https://issues.apache.org/jira/browse/HIVE-7935 > > > Diffs > ----- > > common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 31aeba9 > itests/hive-unit/src/test/java/org/apache/hive/jdbc/TestJdbcDriver2.java > ae128a9 > jdbc/src/java/org/apache/hive/jdbc/HiveConnection.java cbcfec7 > jdbc/src/java/org/apache/hive/jdbc/HiveDriver.java 6e248d6 > jdbc/src/java/org/apache/hive/jdbc/JdbcUriParseException.java PRE-CREATION > jdbc/src/java/org/apache/hive/jdbc/Utils.java 58339bf > jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientException.java > PRE-CREATION > jdbc/src/java/org/apache/hive/jdbc/ZooKeeperHiveClientHelper.java > PRE-CREATION > > ql/src/java/org/apache/hadoop/hive/ql/lockmgr/zookeeper/ZooKeeperHiveLockManager.java > 0919d2f > ql/src/java/org/apache/hadoop/hive/ql/util/ZooKeeperHiveHelper.java > PRE-CREATION > > ql/src/test/org/apache/hadoop/hive/ql/lockmgr/zookeeper/TestZookeeperLockManager.java > 59294b1 > service/src/java/org/apache/hive/service/cli/CLIService.java a0bc905 > > service/src/java/org/apache/hive/service/cli/operation/OperationManager.java > f5a8f27 > service/src/java/org/apache/hive/service/cli/session/HiveSessionImpl.java > b0bb8be > service/src/java/org/apache/hive/service/cli/session/SessionManager.java > 11d25cc > > service/src/java/org/apache/hive/service/cli/thrift/ThriftBinaryCLIService.java > 2b80adc > service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java > 443c371 > > service/src/java/org/apache/hive/service/cli/thrift/ThriftHttpCLIService.java > 4067106 > service/src/java/org/apache/hive/service/server/HiveServer2.java 124996c > > service/src/test/org/apache/hive/service/cli/session/TestSessionGlobalInitFile.java > 66fc1fc > > Diff: https://reviews.apache.org/r/25245/diff/ > > > Testing > ------- > > Manual testing. > > > Thanks, > > Vaibhav Gumashta > >
