> On Sept. 2, 2016, 9:32 p.m., Gabor Szadovszky wrote: > > beeline/src/java/org/apache/hive/beeline/BeeLine.java, line 932 > > <https://reviews.apache.org/r/51593/diff/1/?file=1490492#file1490492line932> > > > > Do we need this one to be public? Seems that is only used inside this > > class. > > Please, consider narrowing the visibility of the other new methods in > > this class.
Unfortunately, I cannot change some of the methods to private or default (package) visibility. The only two methods getDefaultHS2ConnectionFileParser and getHiveSiteHS2ConnectionFileParser need to be public for improving the test coverage. I tried to move them in another package and create a factory class to get the HS2ConnectionParsers, but hit the wall when I tried to mock static methods. The code was becoming convuluted and I thought it is best not to go further that route in the interest of code readability. I have annotated them @VisibleForTesting to document their increased visibility. Let me know if you have any better ideas. > On Sept. 2, 2016, 9:32 p.m., Gabor Szadovszky wrote: > > itests/hive-unit/src/test/java/org/apache/hive/beeline/hs2connectionfile/TestBeelineWithHS2ConnectionFile.java, > > line 190 > > <https://reviews.apache.org/r/51593/diff/1/?file=1490509#file1490509line190> > > > > It seems that the last 3 methods have some common code. I would > > consider moving the common code parts to one place. Modified existing method with added arguments so that other two can be get ridden of. - Vihang ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51593/#review147678 ----------------------------------------------------------- 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 > >
