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