ZihanLi58 commented on code in PR #3804:
URL: https://github.com/apache/gobblin/pull/3804#discussion_r1370633244
##########
gobblin-data-management/src/main/java/org/apache/gobblin/data/management/copy/publisher/CopyDataPublisher.java:
##########
@@ -300,6 +300,10 @@ private void publishFileSet(CopyEntity.DatasetAndPartition
datasetAndPartition,
}
}
+ // execute post publish commit steps after preserving file attributes,
because some post publish step,
+ // e.g. SetPermissionCommitStep needs to set permissions
+ executeCommitSequence(postPublish);
Review Comment:
We do move path from tmp to dst in this step, I believe it should happen
before the set file attribute. We should make sure set file attribute preserve
the right permission
##########
gobblin-data-management/src/main/java/org/apache/gobblin/data/management/copy/ManifestBasedDataset.java:
##########
@@ -128,6 +143,12 @@ public Iterator<FileSet<CopyEntity>>
getFileSetIterator(FileSystem targetFs, Cop
toDelete.add(targetFs.getFileStatus(fileToCopy));
}
}
+
+ Properties props = new Properties();
+ props.setProperty(SetPermissionCommitStep.STOP_ON_ERROR_KEY, "true");
+ CommitStep setPermissionCommitStep = new
SetPermissionCommitStep(targetFs, ancestorOwnerAndPermissions, props);
Review Comment:
Why do we need this step? if dir already exist on dst and user want to
change the permission for that dir, it will appear in the manifest? If dir not
exist, we should directly create parent dir to be the same as source dir?
--
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]
