----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51593/#review148007 -----------------------------------------------------------
Hi Vihang, Finally, I had time to read through the code. One question about the requirements, and few smaller ones. Otherwise LGTM. Thanks, Peter beeline/src/java/org/apache/hive/beeline/BeeLine.java (lines 936 - 938) <https://reviews.apache.org/r/51593/#comment215362> I am not sure about this, and this is more like a requirement question, than a coding question: - We might like to use the hive-site.xml based connection data, even there is no user specific connection data. For example: - We have an unsecured cluster, so we do not mind, if everyone uses the same username, password using hive. This might be a valid usecase. We still can chose not to support this, since this is a bad practice :) So this is just a question, nothing more! beeline/src/java/org/apache/hive/beeline/BeeLine.java (lines 957 - 958) <https://reviews.apache.org/r/51593/#comment215366> Public + VisibleForTesting beeline/src/java/org/apache/hive/beeline/BeeLine.java (lines 962 - 963) <https://reviews.apache.org/r/51593/#comment215367> Public + VisibleForTesting beeline/src/java/org/apache/hive/beeline/hs2connection/HS2ConnectionFileParser.java (line 88) <https://reviews.apache.org/r/51593/#comment215364> I might prefer not to expose the file location on the interface, if it is not absolutely required. What do you think of using instead of this a new public method: public boolean isConfigExists()? - Peter Vary On Sept. 6, 2016, 11:53 p.m., Vihang Karajgaonkar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51593/ > ----------------------------------------------------------- > > (Updated Sept. 6, 2016, 11:53 p.m.) > > > Review request for hive, Mohit Sabharwal, Sergio Pena, and Szehon Ho. > > > Bugs: HIVE-14063 > https://issues.apache.org/jira/browse/HIVE-14063 > > > Repository: hive-git > > > Description > ------- > > This change adds a new optional configuration file for Beeline. If this file > is present at predefined locations, Beeline will attempt to create the > connection url using the hive-site.xml found in classpath and another > user-specific configuration file. Beeline then connects automatically using > the url generated based on these configuration files. The main objective of > the change is to provide user another way to connect to the HiveServer2 > without providing the connection url everytime. The configuration file uses > hadoop xml format so that we can support encryption/obfuscation using hadoop > credential manager API in the future. > > Properties in the user-specific configuration file override the properties > derived from hive-site.xml. > > Tested using newly added unit tests and itests in various Hiveserver2 > configurations. > > > Diffs > ----- > > beeline/src/java/org/apache/hive/beeline/BeeLine.java > 8e65e3987398531cce5c65c383762cf49a52c578 > beeline/src/java/org/apache/hive/beeline/Commands.java > 2f3ec134098dfa3767bab9545438d1f38f11697c > > beeline/src/java/org/apache/hive/beeline/hs2connection/BeelineHS2ConnectionFileParseException.java > PRE-CREATION > > beeline/src/java/org/apache/hive/beeline/hs2connection/HS2ConnectionFileParser.java > PRE-CREATION > > beeline/src/java/org/apache/hive/beeline/hs2connection/HS2ConnectionFileUtils.java > PRE-CREATION > > beeline/src/java/org/apache/hive/beeline/hs2connection/HiveSiteHS2ConnectionFileParser.java > PRE-CREATION > > beeline/src/java/org/apache/hive/beeline/hs2connection/UserHS2ConnectionFileParser.java > PRE-CREATION > > beeline/src/test/org/apache/hive/beeline/hs2connection/TestUserHS2ConnectionFileParser.java > PRE-CREATION > beeline/src/test/resources/hive-site.xml > 5f310d68245275ac9dc24df45579784019eea332 > beeline/src/test/resources/test-hs2-conn-conf-kerberos-http.xml > PRE-CREATION > beeline/src/test/resources/test-hs2-conn-conf-kerberos-nossl.xml > PRE-CREATION > beeline/src/test/resources/test-hs2-conn-conf-kerberos-ssl.xml PRE-CREATION > beeline/src/test/resources/test-hs2-connection-conf-list.xml PRE-CREATION > beeline/src/test/resources/test-hs2-connection-config-noauth.xml > PRE-CREATION > beeline/src/test/resources/test-hs2-connection-multi-conf-list.xml > PRE-CREATION > beeline/src/test/resources/test-hs2-connection-zookeeper-config.xml > PRE-CREATION > > itests/hive-unit/src/test/java/org/apache/hive/beeline/hs2connection/TestBeelineConnectionUsingHiveSite.java > PRE-CREATION > > itests/hive-unit/src/test/java/org/apache/hive/beeline/hs2connection/TestBeelineWithHS2ConnectionFile.java > PRE-CREATION > > itests/hive-unit/src/test/java/org/apache/hive/beeline/hs2connection/TestBeelineWithUserHs2ConnectionFile.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/51593/diff/ > > > Testing > ------- > > > Thanks, > > Vihang Karajgaonkar > >
