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:56 GMT


> On Sept. 2, 2016, 10:06 p.m., Peter Vary wrote:
> > beeline/src/java/org/apache/hive/beeline/BeeLine.java, line 953
> > <https://reviews.apache.org/r/51593/diff/1/?file=1490492#file1490492line953>
> >
> >     nit: missing space if (

fixed


> On Sept. 2, 2016, 10:06 p.m., Peter Vary wrote:
> > beeline/src/java/org/apache/hive/beeline/BeeLine.java, line 960
> > <https://reviews.apache.org/r/51593/diff/1/?file=1490492#file1490492line960>
> >
> >     What happens if there is not hive-site.xml on the classpath?

If the hive-site.xml is not a in the classpath, this method should return an empty properties
object and only user specific configuration will be used. Thanks for pointing it out. I made
some changes in getConnectionProperties in HiveSiteHS2ConnectionFileParser to address the
same.


> On Sept. 2, 2016, 10:06 p.m., Peter Vary wrote:
> > beeline/src/java/org/apache/hive/beeline/hs2connection/HiveSiteHS2ConnectionFileParser.java,
line 42
> > <https://reviews.apache.org/r/51593/diff/1/?file=1490497#file1490497line42>
> >
> >     Should not this be static?

fixed


> On Sept. 2, 2016, 10:06 p.m., Peter Vary wrote:
> > beeline/src/java/org/apache/hive/beeline/hs2connection/HS2ConnectionFileUtils.java,
line 34
> > <https://reviews.apache.org/r/51593/diff/1/?file=1490496#file1490496line34>
> >
> >     Should not we use constants here?

I think for the new ones keys which this features we can add them as constants in the interface
so that anyone implementing them can use them. But for the other keys in the url, there are
too many of them and it can increase over time. I don't think we should use constants for
them.


> On Sept. 2, 2016, 10:06 p.m., Peter Vary wrote:
> > beeline/src/java/org/apache/hive/beeline/hs2connection/HiveSiteHS2ConnectionFileParser.java,
line 57
> > <https://reviews.apache.org/r/51593/diff/1/?file=1490497#file1490497line57>
> >
> >     It this is public, do we need the VisibleForTesting annotation?

fixed


- Vihang


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/51593/#review147700
-----------------------------------------------------------


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