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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
