hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Vihang Karajgaonkar <vih...@cloudera.com>
Subject Re: Review Request 51593: HIVE-14063 : Add beeline connection configuration file to automatically connect to HS2 using beeline
Date Tue, 06 Sep 2016 23:53:50 GMT


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


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message