[GitHub] [hadoop] omalley commented on a change in pull request #3956: HADOOP-18110. ViewFileSystem: Add Support for Localized Trash Root

[GitBox <gi...@apache.org>]

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: common-issues-unsubscribe@hadoop.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: common-issues-unsubscribe@hadoop.apache.org
For additional commands, e-mail: common-issues-help@hadoop.apache.org