sadanand48 commented on code in PR #5364:
URL: https://github.com/apache/hadoop/pull/5364#discussion_r1138906146
##########
hadoop-tools/hadoop-distcp/src/test/java/org/apache/hadoop/tools/TestDistCpSync.java:
##########
@@ -1276,4 +1283,63 @@ private void snapshotDiffWithPaths(Path sourceFSPath,
verifyCopyByFs(sourceFS, targetFS, sourceFS.getFileStatus(sourceFSPath),
targetFS.getFileStatus(targetFSPath), false);
}
+
+ @Test
+ public void testSyncSnapshotDiffWithLocalFileSystem() throws Exception {
+ String[] args = new String[]{"-update", "-diff", "s1", "s2",
+ "file:///source", "file:///target"};
+ LambdaTestUtils.intercept(
+ IllegalArgumentException.class,
+ "The source file system file does not support snapshot",
+ () -> new DistCp(conf, OptionsParser.parse(args)).execute());
+ }
+
+ @Test
+ public void testSyncSnapshotDiffWithDummyFileSystem() throws Exception {
+ String[] args =
+ new String[] { "-update", "-diff", "s1", "s2", "dummy:///source",
+ "dummy:///target" };
+ try {
+ FileSystem dummyFs = FileSystem.get(URI.create("dummy:///"), conf);
+ Assert.assertTrue(dummyFs instanceof DummyFs);
+ new DistCp(conf, OptionsParser.parse(args)).execute();
+ } catch (Exception e) {
+ // can expect other exceptions as source and target paths
+ // are not created, assert if the exception is not arising
+ // due to the filesystem not supporting snapshots.
+ Assert.assertFalse(e.getMessage().contains("does not support snapshot"));
Review Comment:
Here I'm checking that a particular exception i.e exception stating that 'fs
doesn't support snapshots' is **not** thrown, The intercept utility method
checks whether the caught exception has a particular text, I think there is no
util method to check its negation.
##########
hadoop-tools/hadoop-distcp/src/main/java/org/apache/hadoop/tools/DistCpSync.java:
##########
@@ -169,20 +153,27 @@ protected boolean preSyncCheck() throws IOException {
return true;
}
- protected void checkFilesystemSupport(FileSystem srcFs, FileSystem tgtFs) {
- // currently we require both the source and the target file system are
- // DistributedFileSystem or (S)WebHdfsFileSystem.
- if (!(srcFs instanceof DistributedFileSystem
- || srcFs instanceof WebHdfsFileSystem)) {
- throw new IllegalArgumentException("Unsupported source file system: "
- + srcFs.getScheme() + "://. " +
- "Supported file systems: hdfs://, webhdfs:// and swebhdfs://.");
+ /**
+ * Check if the source and target filesystems support snapshots.
+ */
+ protected void checkFilesystemSupport(Path sourceDir, Path targetDir,
+ FileSystem srcFs, FileSystem tgtFs) throws IOException {
+ if (!srcFs.hasPathCapability(sourceDir,
+ CommonPathCapabilities.FS_SNAPSHOTS)) {
+ throw new IllegalArgumentException(
+ "The source file system " + srcFs.getScheme() + " does not support
snapshot.");
+ }
+ if (!tgtFs.hasPathCapability(targetDir,
+ CommonPathCapabilities.FS_SNAPSHOTS)) {
+ throw new IllegalArgumentException(
+ "The target file system " + tgtFs.getScheme() + " does not support
snapshot.");
}
- if (!(tgtFs instanceof DistributedFileSystem
- || tgtFs instanceof WebHdfsFileSystem)) {
- throw new IllegalArgumentException("Unsupported target file system: "
- + tgtFs.getScheme() + "://. " +
- "Supported file systems: hdfs://, webhdfs:// and swebhdfs://.");
+ try {
+ getSnapshotDiffReportMethod(srcFs);
+ getSnapshotDiffReportMethod(tgtFs);
+ } catch (NoSuchMethodException e) {
+ throw new IllegalArgumentException(
Review Comment:
Done.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
