> On 2011-04-05 07:52:15, Amareshwari Sriramadasu wrote: > > http://svn.apache.org/repos/asf/hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java, > > line 152 > > <https://reviews.apache.org/r/528/diff/2/?file=14844#file14844line152> > > > > HadoopShims.isSecureShimImpl() is not called anywhere else. Shall we > > remove it if not required anymore?
I suggest we leave it there. This seems like a useful method, and I am actually using it in another patch. > On 2011-04-05 07:52:15, Amareshwari Sriramadasu wrote: > > http://svn.apache.org/repos/asf/hive/trunk/shims/src/test/org/apache/hadoop/hive/thrift/TestHadoop20SAuthBridge.java, > > lines 144-156 > > <https://reviews.apache.org/r/528/diff/2/?file=14850#file14850line144> > > > > Do you want to move this into setup(), as it is common in both > > testcases? Done > On 2011-04-05 07:52:15, Amareshwari Sriramadasu wrote: > > http://svn.apache.org/repos/asf/hive/trunk/shims/src/test/org/apache/hadoop/hive/thrift/TestHadoop20SAuthBridge.java, > > lines 192-209 > > <https://reviews.apache.org/r/528/diff/2/?file=14850#file14850line192> > > > > code looks duplicated. Can it be refactored by passing group names to a > > method? Done - Devaraj ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/528/#review386 ----------------------------------------------------------- On 2011-03-29 10:26:38, Devaraj Das wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/528/ > ----------------------------------------------------------- > > (Updated 2011-03-29 10:26:38) > > > Review request for hive. > > > Summary > ------- > > Fixes to some security issues discussed in HIVE-1988 > > > This addresses bug HIVE-1988. > https://issues.apache.org/jira/browse/HIVE-1988 > > > Diffs > ----- > > > http://svn.apache.org/repos/asf/hive/trunk/metastore/if/hive_metastore.thrift > 1085623 > > http://svn.apache.org/repos/asf/hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java > 1085623 > > http://svn.apache.org/repos/asf/hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStoreClient.java > 1085623 > > http://svn.apache.org/repos/asf/hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/IMetaStoreClient.java > 1085623 > > http://svn.apache.org/repos/asf/hive/trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/DelegationTokenSecretManager.java > 1085623 > > http://svn.apache.org/repos/asf/hive/trunk/shims/src/0.20S/java/org/apache/hadoop/hive/thrift/HadoopThriftAuthBridge20S.java > 1085623 > > http://svn.apache.org/repos/asf/hive/trunk/shims/src/common/java/org/apache/hadoop/hive/thrift/HadoopThriftAuthBridge.java > 1085623 > > http://svn.apache.org/repos/asf/hive/trunk/shims/src/test/org/apache/hadoop/hive/thrift/TestHadoop20SAuthBridge.java > 1085623 > > Diff: https://reviews.apache.org/r/528/diff > > > Testing > ------- > > New unit test added and that passes. All unit tests passed. > > > Thanks, > > Devaraj > >
