phet commented on code in PR #3845:
URL: https://github.com/apache/gobblin/pull/3845#discussion_r1424664958
##########
gobblin-data-management/src/main/java/org/apache/gobblin/data/management/copy/ManifestBasedDataset.java:
##########
@@ -176,10 +176,11 @@ public Iterator<FileSet<CopyEntity>>
getFileSetIterator(FileSystem targetFs, Cop
private static boolean shouldCopy(FileSystem targetFs, FileStatus
fileInSource, FileStatus fileInTarget, OwnerAndPermission replicatedPermission)
throws IOException {
- if (fileInSource.isDirectory() || fileInSource.getModificationTime() ==
fileInTarget.getModificationTime()) {
- // if source is dir or source and dst has same version, we compare the
permission to determine whether it needs another sync
+ if (fileInSource.isDirectory() || fileInSource.getModificationTime() <=
fileInTarget.getModificationTime()) {
+ // even if destination has a newer version than the source, we still
copy the file if the owner or permission is different
return !replicatedPermission.hasSameOwnerAndPermission(targetFs,
fileInTarget);
}
- return fileInSource.getModificationTime() >
fileInTarget.getModificationTime();
+ // Source is newer than the target, must copy
+ return true;
Review Comment:
would this be equivalent:
```
return fileInSrc.modT() > fileInTarg.modT() ||
!replicatedPermission.hasSameOwnerAndPerms(targFs, fileInTarget);
```
?
##########
gobblin-data-management/src/main/java/org/apache/gobblin/data/management/copy/ManifestBasedDataset.java:
##########
@@ -176,10 +176,11 @@ public Iterator<FileSet<CopyEntity>>
getFileSetIterator(FileSystem targetFs, Cop
private static boolean shouldCopy(FileSystem targetFs, FileStatus
fileInSource, FileStatus fileInTarget, OwnerAndPermission replicatedPermission)
throws IOException {
- if (fileInSource.isDirectory() || fileInSource.getModificationTime() ==
fileInTarget.getModificationTime()) {
- // if source is dir or source and dst has same version, we compare the
permission to determine whether it needs another sync
+ if (fileInSource.isDirectory() || fileInSource.getModificationTime() <=
fileInTarget.getModificationTime()) {
+ // even if destination has a newer version than the source, we still
copy the file if the owner or permission is different
return !replicatedPermission.hasSameOwnerAndPermission(targetFs,
fileInTarget);
}
- return fileInSource.getModificationTime() >
fileInTarget.getModificationTime();
+ // Source is newer than the target, must copy
+ return true;
Review Comment:
would this be equivalent:
```
return fileInSrc.modT() > fileInTarg.modT()
|| !replicatedPermission.hasSameOwnerAndPerms(targFs, fileInTarget);
```
?
--
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]