hive-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ashish Singh" <asi...@cloudera.com>
Subject Re: Review Request 22016: HIVE-7119 Extended ACL's should be inherited if warehouse perm inheritance enabled
Date Thu, 29 May 2014 20:28:46 GMT

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



ql/src/java/org/apache/hadoop/hive/ql/processors/DeleteResourceProcessor.java
<https://reviews.apache.org/r/22016/#comment78625>

    Is this here by mistake?



shims/0.20/src/main/java/org/apache/hadoop/hive/shims/Hadoop20Shims.java
<https://reviews.apache.org/r/22016/#comment78630>

    Shouldn't we be trying to change permission even if we fail to set group? Also, if I remember
correctly, trying to set group was leading to exceptions.
    



shims/0.20S/src/main/java/org/apache/hadoop/hive/shims/Hadoop20SShims.java
<https://reviews.apache.org/r/22016/#comment78633>

    Same as previous comment regarding settign group.



shims/0.20S/src/main/java/org/apache/hadoop/hive/shims/Hadoop20SShims.java
<https://reviews.apache.org/r/22016/#comment78635>

    This is similar to Hadoop20FileStatus. Could we push them together into a base class?



shims/0.23/src/main/java/org/apache/hadoop/hive/shims/Hadoop23Shims.java
<https://reviews.apache.org/r/22016/#comment78637>

    Objects.equal(conf.get("dfs.namenode.acls.enabled"), "true") is used at more than one
place. Should we extract it out to a static method?



shims/0.23/src/main/java/org/apache/hadoop/hive/shims/Hadoop23Shims.java
<https://reviews.apache.org/r/22016/#comment78636>

    Same as previous comment on setting group.


- Ashish Singh


On May 29, 2014, 8:14 a.m., Szehon Ho wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22016/
> -----------------------------------------------------------
> 
> (Updated May 29, 2014, 8:14 a.m.)
> 
> 
> Review request for hive.
> 
> 
> Bugs: HIVE-7119
>     https://issues.apache.org/jira/browse/HIVE-7119
> 
> 
> Repository: hive-git
> 
> 
> Description
> -------
> 
> This completes the permission inheritance story, by also inheriting the new concept of
extended ACL's in HDFS from parent.
> 
> This is a bit tricky because only Hadoop 2.4 has extended ACL's.  My strategy is to use
the HadoopShims, and only in Hadoop23Shims to have code dealing with extended ACL's, and then
only if the flag "dfs.namenode.acls.enabled" is true.
> 
> It was also tricky as the main Hive code cannot refer to the HDFS ACL classes (aclStatus
and aclEntry).  So made some wrapper API in the shims called 'hdfsFileStatus' that encompasses
both normal file status , and Aclstatus if acl's are enabled.
> 
> 
> Diffs
> -----
> 
>   common/src/java/org/apache/hadoop/hive/common/FileUtils.java ee61350 
>   itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/FolderPermissionBase.java
PRE-CREATION 
>   itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestExtendedAcls.java
PRE-CREATION 
>   itests/hive-unit/src/test/java/org/apache/hadoop/hive/ql/security/TestFolderPermissions.java
4f566d2 
>   ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java 3417474 
>   ql/src/java/org/apache/hadoop/hive/ql/processors/DeleteResourceProcessor.java 83fadeb

>   shims/0.20/src/main/java/org/apache/hadoop/hive/shims/Hadoop20Shims.java 7aae689 
>   shims/0.20S/src/main/java/org/apache/hadoop/hive/shims/Hadoop20SShims.java e6493eb

>   shims/0.23/src/main/java/org/apache/hadoop/hive/shims/Hadoop23Shims.java 176f9ae 
>   shims/common-secure/src/main/java/org/apache/hadoop/hive/shims/HadoopShimsSecure.java
c27df64 
>   shims/common/src/main/java/org/apache/hadoop/hive/shims/HadoopShims.java 02260ce 
> 
> Diff: https://reviews.apache.org/r/22016/diff/
> 
> 
> Testing
> -------
> 
> For testing, refactored TestFolderPermission into a base + two tests (TestFolderPermission
to test traditional permission without acl's, and TestExtendedAcl's to test acls).
> 
> 
> Thanks,
> 
> Szehon Ho
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message