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 Wed, 07 Sep 2016 18:23:06 GMT


> On Sept. 7, 2016, 2:38 p.m., Peter Vary wrote:
> > beeline/src/java/org/apache/hive/beeline/hs2connection/HS2ConnectionFileParser.java,
line 88
> > <https://reviews.apache.org/r/51593/diff/2/?file=1492642#file1492642line88>
> >
> >     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()?

Good idea. Added a new interface method configExists() instead of getFileLocation.


> On Sept. 7, 2016, 2:38 p.m., Peter Vary wrote:
> > beeline/src/java/org/apache/hive/beeline/BeeLine.java, lines 981-982
> > <https://reviews.apache.org/r/51593/diff/2/?file=1492639#file1492639line981>
> >
> >     Public + VisibleForTesting

This is a marker annotation for documetation purposes only. The method needs to be more visible
than required only for test coverage. I have added a comment saying the same


> On Sept. 7, 2016, 2:38 p.m., Peter Vary wrote:
> > beeline/src/java/org/apache/hive/beeline/BeeLine.java, lines 976-977
> > <https://reviews.apache.org/r/51593/diff/2/?file=1492639#file1492639line976>
> >
> >     Public + VisibleForTesting

This is a marker annotation for documetation purposes only. The method needs to be more visible
than required only for test coverage. I have added a comment saying the same


> On Sept. 7, 2016, 2:38 p.m., Peter Vary wrote:
> > beeline/src/java/org/apache/hive/beeline/BeeLine.java, lines 955-957
> > <https://reviews.apache.org/r/51593/diff/2/?file=1492639#file1492639line955>
> >
> >     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!

I had thought about this particular case. The issue is this change needs to be backwards compatible
which means that previous methods of connecting using url should work. That means Beeline
should know using some flag to use hive-site.xml or not. The presence of this file indicates
that hive-site.xml is used or not. I had initially added an explicit flag for the same in
user conf file, but seemed redundant to me. So if user would like to use only hive-site.xml
for connection info, the can provide an empty file and it will serve as a flag to look for
hive-site.xml. This may be true in a secure, kerberos enabled cluster where hive-site.xml
has all the information that we need to connect. But there is a bug where beeline still asks
for username, password even in kerberos enabled cluster, so currently, even in that case there
needs to be some user-specific information which needs to be provided. I am planning to fix
that case in a separate change. For all the other changes, there needs t
 o be a non-empty user-specific file which atleast provides user/password since hive-site.xml
doesn't have that information.


- Vihang


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


On Sept. 7, 2016, 6:22 p.m., Vihang Karajgaonkar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/51593/
> -----------------------------------------------------------
> 
> (Updated Sept. 7, 2016, 6:22 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