[GitHub] [gobblin] Will-Lo commented on a diff in pull request #3616: [GOBBLIN-1755] Support extended ACLs and sticky bit for file based distcp

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

Will-Lo commented on code in PR #3616:
URL: https://github.com/apache/gobblin/pull/3616#discussion_r1069681531


##########
gobblin-data-management/src/main/java/org/apache/gobblin/data/management/copy/writer/FileAwareInputStreamDataWriter.java:
##########
@@ -500,15 +504,26 @@ private void ensureDirectoryExists(FileSystem fs, Path path, Iterator<OwnerAndPe
 
       String group = ownerAndPermission.getGroup();
       String owner = ownerAndPermission.getOwner();
+      List<AclEntry> aclEntries = ownerAndPermission.getAclEntries();
       if (group != null || owner != null) {
         log.debug("Applying owner {} and group {} to path {}.", owner, group, path);
         fs.setOwner(path, owner, group);
       }
+      if (!aclEntries.isEmpty()) {
+        setAclOnPath(fs, path, aclEntries);
+      }
     } else {
       fs.mkdirs(path);
     }
   }
 
+  /*
+   * Creating an abstraction layer to support unit testing on raw local file system for validating if ACLs are set on a file path
+   */
+  protected void setAclOnPath(FileSystem theFs, Path path, List<AclEntry> aclEntries) throws IOException {
+    theFs.setAcl(path, aclEntries);
+  }

Review Comment:
   Question, does LocalFileSystem in Hadoop support setAcl and a reader on acls? It could be more straightforward to test directly using set and get acls on a random file you write locally. 
   
   If not this is fine



-- 
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: dev-unsubscribe@gobblin.apache.org

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