----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/24170/#review49375 -----------------------------------------------------------
common/src/java/org/apache/hadoop/hive/common/FileUtils.java <https://reviews.apache.org/r/24170/#comment86378> do we need to have "Exception" class in the throws ? Shims.doAs() throws IOException, InterruptedException common/src/java/org/apache/hadoop/hive/common/FileUtils.java <https://reviews.apache.org/r/24170/#comment86375> I assume this actually throws some subclasses of exception, can you change the throws to specify those subclasses instead of exception ? itests/hive-unit-hadoop2/src/test/java/org/apache/hadoop/hive/ql/security/TestStorageBasedMetastoreAuthorizationProviderWithACL.java <https://reviews.apache.org/r/24170/#comment86386> make it private ? shims/common/src/main/java/org/apache/hadoop/fs/DefaultFileAccess.java <https://reviews.apache.org/r/24170/#comment86381> I think we should add an else here, to make it actually compatible with HDFS/POSIX way of checking permissions. shims/common/src/main/java/org/apache/hadoop/fs/DefaultFileAccess.java <https://reviews.apache.org/r/24170/#comment86383> another else here as well shims/common/src/main/java/org/apache/hadoop/hive/shims/HadoopShims.java <https://reviews.apache.org/r/24170/#comment86385> I think we should remove the user, and groups from the arguments, and make it compatible with the arguments of access method. If the group is different from what HDFS is going to see, then we are not really checking against the HDFS permissions. Keeping the metadata authorization in sync with HDFS permissions is goal of SBA. So nobody should be using a authentication provider that is inconsistent with HDFS groups, and I don't think anybody would be doing that. We are also ignoring those arguments when the newer versions of hadoop get used. I think removing the arguments from here will make that more clear to other developers, and is better for longer term code cleanliness. - Thejas Nair On Aug. 1, 2014, 12:54 a.m., Jason Dere wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/24170/ > ----------------------------------------------------------- > > (Updated Aug. 1, 2014, 12:54 a.m.) > > > Review request for hive and Thejas Nair. > > > Bugs: HIVE-7583 > https://issues.apache.org/jira/browse/HIVE-7583 > > > Repository: hive-git > > > Description > ------- > > Create shims method to either use new FileSystem.access() API, or fall back > to Hive implementation of file access checks. > > > Diffs > ----- > > common/src/java/org/apache/hadoop/hive/common/FileUtils.java 8dcd4cf > > itests/hive-unit-hadoop2/src/test/java/org/apache/hadoop/hive/ql/security/TestStorageBasedMetastoreAuthorizationProviderWithACL.java > PRE-CREATION > > itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestMetastoreAuthorizationProvider.java > 97fb7ba > > itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestStorageBasedMetastoreAuthorizationProvider.java > 223f155 > > ql/src/java/org/apache/hadoop/hive/ql/security/authorization/StorageBasedAuthorizationProvider.java > f803cc4 > > ql/src/java/org/apache/hadoop/hive/ql/security/authorization/plugin/sqlstd/SQLAuthorizationUtils.java > 6a283ab > shims/0.20/src/main/java/org/apache/hadoop/hive/shims/Hadoop20Shims.java > d03f270 > shims/0.23/src/main/java/org/apache/hadoop/hive/shims/Hadoop23Shims.java > e98b54d > > shims/common-secure/src/main/java/org/apache/hadoop/hive/shims/HadoopShimsSecure.java > 605ea55 > shims/common/src/main/java/org/apache/hadoop/fs/DefaultFileAccess.java > PRE-CREATION > shims/common/src/main/java/org/apache/hadoop/hive/shims/HadoopShims.java > eefd5e5 > > Diff: https://reviews.apache.org/r/24170/diff/ > > > Testing > ------- > > > Thanks, > > Jason Dere > >
