omalley commented on a change in pull request #3956:
URL: https://github.com/apache/hadoop/pull/3956#discussion_r803178289



##########
File path: 
hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/viewfs/ViewFileSystem.java
##########
@@ -1130,16 +1132,49 @@ public BlockStoragePolicySpi getStoragePolicy(Path src) 
throws IOException {
    * Get the trash root directory for current user when the path
    * specified is deleted.
    *
+   * If CONFIG_VIEWFS_MOUNT_POINT_LOCAL_TRASH is not set, return
+   * the default trash root from targetFS.
+   *
+   * When CONFIG_VIEWFS_MOUNT_POINT_LOCAL_TRASH is set to true,
+   * 1) If path p is in fallback FS or from the same mount point as the default
+   *    trash root for targetFS, return the default trash root for targetFS.
+   * 2) else, return a trash root in the mounted targetFS
+   *    (/mntpoint/.Trash/<user>)

Review comment:
       The "<user>" causes javadoc to fail as an unknown tag. You need to quote 
using &lt; and &gt;.

##########
File path: 
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/viewfs/ViewFileSystemBaseTest.java
##########
@@ -1102,6 +1104,131 @@ public void testTrashRoot() throws IOException {
     Assert.assertTrue("", fsView.getTrashRoots(true).size() > 0);
   }
 
+  /**

Review comment:
       Please add a test where the child file system returns a root inside of 
the mount point, such as what will happen when there is an encryption zone 
root. To do so, you'll need to define a mock/stub fs.




-- 
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]

Reply via email to