[GitHub] [gobblin] ZihanLi58 commented on a diff in pull request #3624: [GOBBLIN-1765] Add support to sync metadata for dir in manifest based copy

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

ZihanLi58 commented on code in PR #3624:
URL: https://github.com/apache/gobblin/pull/3624#discussion_r1081946270


##########
gobblin-data-management/src/main/java/org/apache/gobblin/data/management/copy/CopyableFile.java:
##########
@@ -316,23 +316,33 @@ private List<OwnerAndPermission> replicateAncestorsOwnerAndPermission(FileSystem
    */
   public static OwnerAndPermission resolveReplicatedOwnerAndPermission(FileSystem fs, Path path,
       CopyConfiguration copyConfiguration) throws IOException {
-
-    PreserveAttributes preserve = copyConfiguration.getPreserve();
     Optional<FileStatus> originFileStatus = copyConfiguration.getCopyContext().getFileStatus(fs, path);
 
     if (!originFileStatus.isPresent()) {
       throw new IOException(String.format("Origin path %s does not exist.", path));
     }
 
+    return resolveReplicatedOwnerAndPermission(originFileStatus.get(), copyConfiguration);
+  }
+
+  /**
+   * Computes the correct {@link OwnerAndPermission} obtained from replicating source owner and permissions and applying
+   * the {@link PreserveAttributes} rules in copyConfiguration.
+   * @throws IOException
+   */
+  public static OwnerAndPermission resolveReplicatedOwnerAndPermission(FileStatus originFileStatus,
+      CopyConfiguration copyConfiguration) {
+
+    PreserveAttributes preserve = copyConfiguration.getPreserve();

Review Comment:
   I want to use the same existing function to calculate the final permission which is  CopyableFile.resolveReplicatedOwnerAndPermission() so that we won't ends up with different behavior in the future when we modify the logic of permission preserving; That function require the whole copyConfiguration because other than the preserve config, we also support target group config to set the user defined group, and I assume that we might want to support something like this for facl as well in the future. 



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