Home | About | Sematext search-lucene.com search-hadoop.com
 Search Hadoop and all its subprojects:

Switch to Threaded View
Hive >> mail # dev >> Review Request: HIVE-4356 - remove duplicate impersonation parameters for hiveserver2


Copy link to this message
-
Re: Review Request: HIVE-4356 - remove duplicate impersonation parameters for hiveserver2

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

common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
<https://reviews.apache.org/r/10554/#comment41943>

    Why was hive.server2.enable.doAs chosen over hive.server2.enable.impersonation? Also, this behavior was disabled by default in both CDH4 and HDP 1.2. I'm not sure that flipping the switch now is a great idea, and long term (i.e. with fine-grained authorization) this is definitely the wrong default behavior.

service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java
<https://reviews.apache.org/r/10554/#comment41944>

    I think this is a lot less clear now that the username code has been pulled out into a separate method. The name "getUserName" implies that it's extracting this information from TOpenSessionReq when that's not actually the case. Please move this back into OpenSession() and add a comment there if you think it's necessary.

service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java
<https://reviews.apache.org/r/10554/#comment41945>

    Is refactoring this code into a separate protected method really necessary? Every other method in this class is self-contained. I'd like to see that pattern maintained.

service/src/test/org/apache/hive/service/auth/TestPlainSaslHelper.java
<https://reviews.apache.org/r/10554/#comment41946>

    Formatting.

service/src/test/org/apache/hive/service/cli/thrift/TestThriftCLIService.java
<https://reviews.apache.org/r/10554/#comment41947>

    Formatting

service/src/test/org/apache/hive/service/cli/thrift/TestThriftCLIService.java
<https://reviews.apache.org/r/10554/#comment41950>

    Formatting

service/src/test/org/apache/hive/service/cli/thrift/TestThriftCLIService.java
<https://reviews.apache.org/r/10554/#comment41948>

    Formatting

service/src/test/org/apache/hive/service/cli/thrift/TestThriftCLIService.java
<https://reviews.apache.org/r/10554/#comment41949>

    Formatting
- Carl Steinbach
On April 16, 2013, 9:46 p.m., Thejas Nair wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10554/
> -----------------------------------------------------------
>
> (Updated April 16, 2013, 9:46 p.m.)
>
>
> Review request for hive.
>
>
> Description
> -------
>
> remove duplicate impersonation parameters for hiveserver2
>
>
> This addresses bug HIVE-4356.
>     https://issues.apache.org/jira/browse/HIVE-4356
>
>
> Diffs
> -----
>
>   common/src/java/org/apache/hadoop/hive/conf/HiveConf.java 78d9cc9
>   conf/hive-default.xml.template e266ce7
>   service/src/java/org/apache/hive/service/auth/PlainSaslHelper.java 18d4aae
>   service/src/java/org/apache/hive/service/cli/CLIService.java b53599b
>   service/src/java/org/apache/hive/service/cli/thrift/ThriftCLIService.java 43d79aa
>   service/src/test/org/apache/hive/service/auth/TestPlainSaslHelper.java PRE-CREATION
>   service/src/test/org/apache/hive/service/cli/thrift/TestThriftCLIService.java PRE-CREATION
>
> Diff: https://reviews.apache.org/r/10554/diff/
>
>
> Testing
> -------
>
> Unit tests included.
> Manually tested on (kerberos) secure and unsecure cluster.
>
>
> Thanks,
>
> Thejas Nair
>
>