----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51593/#review147700 -----------------------------------------------------------
Thanks for the patch! This will be a really a usefull feature! I did not have time to take a closer look on it. What I have seen is nice, there is some nits, and questions (probably some of them is invalid - not finalized yet). Will have more time next week. beeline/src/java/org/apache/hive/beeline/BeeLine.java (line 934) <https://reviews.apache.org/r/51593/#comment214918> nit: missing space if ( beeline/src/java/org/apache/hive/beeline/BeeLine.java (line 941) <https://reviews.apache.org/r/51593/#comment214917> What happens if there is not hive-site.xml on the classpath? beeline/src/java/org/apache/hive/beeline/hs2connection/HS2ConnectionFileUtils.java (line 34) <https://reviews.apache.org/r/51593/#comment214919> Should not we use constants here? beeline/src/java/org/apache/hive/beeline/hs2connection/HiveSiteHS2ConnectionFileParser.java (line 42) <https://reviews.apache.org/r/51593/#comment214920> Should not this be static? beeline/src/java/org/apache/hive/beeline/hs2connection/HiveSiteHS2ConnectionFileParser.java (line 57) <https://reviews.apache.org/r/51593/#comment214921> It this is public, do we need the VisibleForTesting annotation? Thanks, Peter - Peter Vary On Sept. 2, 2016, 6:30 a.m., Vihang Karajgaonkar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51593/ > ----------------------------------------------------------- > > (Updated Sept. 2, 2016, 6:30 a.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/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/hs2connectionfile/TestBeelineConnectionUsingHiveSite.java > PRE-CREATION > > itests/hive-unit/src/test/java/org/apache/hive/beeline/hs2connectionfile/TestBeelineWithHS2ConnectionFile.java > PRE-CREATION > > itests/hive-unit/src/test/java/org/apache/hive/beeline/hs2connectionfile/TestBeelineWithUserHs2ConnectionFile.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/51593/diff/ > > > Testing > ------- > > > Thanks, > > Vihang Karajgaonkar > >
