[ 
https://issues.apache.org/jira/browse/HADOOP-18109?focusedWorklogId=721317&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-721317
 ]

ASF GitHub Bot logged work on HADOOP-18109:
-------------------------------------------

                Author: ASF GitHub Bot
            Created on: 05/Feb/22 01:18
            Start Date: 05/Feb/22 01:18
    Worklog Time Spent: 10m 
      Work Description: shvachko commented on a change in pull request #3953:
URL: https://github.com/apache/hadoop/pull/3953#discussion_r799904498



##########
File path: 
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/viewfs/TestViewFileSystemHdfs.java
##########
@@ -18,6 +18,9 @@
 package org.apache.hadoop.fs.viewfs;
 
 import java.io.File;
+import org.apache.hadoop.fs.FileStatus;
+import org.apache.hadoop.fs.permission.AclStatus;

Review comment:
       Remove unused import

##########
File path: 
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/viewfs/TestViewFileSystemHdfs.java
##########
@@ -479,4 +485,22 @@ public Object run() throws IOException {
     assertEquals("The owner did not match ", owner, 
userUgi.getShortUserName());
     otherfs.delete(user1Path, false);
   }
+
+  @Test

Review comment:
       This test does not seem to reproduce the problem. If I revert the change 
it still passes. 
   I believe permission = null is replaced by the default permission somewhere 
down in the impl.

##########
File path: 
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/viewfs/TestViewFileSystemHdfs.java
##########
@@ -18,6 +18,9 @@
 package org.apache.hadoop.fs.viewfs;
 
 import java.io.File;
+import org.apache.hadoop.fs.FileStatus;
+import org.apache.hadoop.fs.permission.AclStatus;
+import org.apache.hadoop.fs.permission.AclUtil;

Review comment:
       Remove unused import

##########
File path: 
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java
##########
@@ -1576,7 +1576,7 @@ public boolean mkdirs(Path dir, FsPermission permission)
 
     @Override
     public boolean mkdirs(Path dir) throws IOException {
-      return mkdirs(dir, null);
+      return mkdirs(dir, FsPermission.getDirDefault());

Review comment:
       This makes the override equivalent to the base class implementation. So 
you can just remove this method.

##########
File path: 
hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/fs/viewfs/TestViewFileSystemHdfs.java
##########
@@ -65,6 +68,9 @@
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import static org.apache.hadoop.fs.viewfs.Constants.*;

Review comment:
       Should not do * imports. Just add what is needed.
   And it looks like they are not needed.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Issue Time Tracking
-------------------

    Worklog Id:     (was: 721317)
    Time Spent: 40m  (was: 0.5h)

> Ensure that default permissions of directories under internal ViewFS 
> directories are the same as directories on target filesystems
> ----------------------------------------------------------------------------------------------------------------------------------
>
>                 Key: HADOOP-18109
>                 URL: https://issues.apache.org/jira/browse/HADOOP-18109
>             Project: Hadoop Common
>          Issue Type: Bug
>            Reporter: Chentao Yu
>            Assignee: Chentao Yu
>            Priority: Major
>              Labels: pull-request-available
>          Time Spent: 40m
>  Remaining Estimate: 0h
>
> * Ensure that default permissions of directories under internal ViewFS 
> directories are the same as directories on target filesystems
>  * Add new unit test



--
This message was sent by Atlassian Jira
(v8.20.1#820001)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to