address diff comment

walterddr · walterddr · commit 4eb336cd24c4 · 2022-02-19T07:41:08.000-08:00
diff --git a/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/PeerDownloadLLCRealtimeClusterIntegrationTest.java b/pinot-integration-tests/src/test/java/org/apache/pinot/integration/tests/PeerDownloadLLCRealtimeClusterIntegrationTest.java
@@ -288,7 +288,7 @@ public boolean doMove(URI srcUri, URI dstUri)
     }
 
     @Override
-    public boolean copy(URI srcUri, URI dstUri)
+    public boolean copyDir(URI srcUri, URI dstUri)
         throws IOException {
       try {
         return _localPinotFS.copyDir(new URI(_basePath + srcUri.getPath()), new URI(_basePath + dstUri.getPath()));
diff --git a/pinot-plugins/pinot-file-system/pinot-adls/src/main/java/org/apache/pinot/plugin/filesystem/AzurePinotFS.java b/pinot-plugins/pinot-file-system/pinot-adls/src/main/java/org/apache/pinot/plugin/filesystem/AzurePinotFS.java
@@ -109,6 +109,7 @@ public boolean doMove(URI srcUri, URI dstUri)
   @Override
   public boolean copyDir(URI srcUri, URI dstUri)
       throws IOException {
+    // TODO: support directory copy.
     if (isDirectory(srcUri)) {
       throw new UnsupportedOperationException("Azure FS doesn't support directory recursive copy!");
     }
diff --git a/pinot-spi/src/main/java/org/apache/pinot/spi/filesystem/PinotFS.java b/pinot-spi/src/main/java/org/apache/pinot/spi/filesystem/PinotFS.java
@@ -103,7 +103,7 @@ boolean move(URI srcUri, URI dstUri, boolean overwrite)
   default boolean copy(URI srcUri, URI dstUri)
       throws IOException {
     if (isDirectory(srcUri)) {
-      throw new UnsupportedOperationException("Recursive copy not supported");
+      throw new IllegalArgumentException("Recursive copy not supported");
     }
     return copyDir(srcUri, dstUri);
   }
@@ -123,10 +123,8 @@ default boolean copy(URI srcUri, URI dstUri)
    * @return true if copy is successful
    * @throws IOException on IO failure
    */
-  default boolean copyDir(URI srcUri, URI dstUri)
-      throws IOException {
-    throw new UnsupportedOperationException("Recursive copy not supported");
-  }
+  boolean copyDir(URI srcUri, URI dstUri)
+      throws IOException;
 
   /**
    * Checks whether the file or directory at the provided location exists.
diff --git a/pinot-spi/src/test/java/org/apache/pinot/spi/filesystem/LocalPinotFSTest.java b/pinot-spi/src/test/java/org/apache/pinot/spi/filesystem/LocalPinotFSTest.java
@@ -209,7 +209,7 @@ public void testFS()
     try {
       localPinotFS.copy(firstTempDir.toURI(), secondTempDir.toURI());
       fail();
-    } catch (UnsupportedOperationException e) {
+    } catch (IllegalArgumentException e) {
       // expected.
     }
 

Original file line number	Diff line number	Diff line change
`@@ -288,7 +288,7 @@ public boolean doMove(URI srcUri, URI dstUri)`
`288`	`288`	`}`
`289`	`289`
`290`	`290`	`@Override`
`291`		`- public boolean copy(URI srcUri, URI dstUri)`
	`291`	`+ public boolean copyDir(URI srcUri, URI dstUri)`
`292`	`292`	`throws IOException {`
`293`	`293`	`try {`
`294`	`294`	`return _localPinotFS.copyDir(new URI(_basePath + srcUri.getPath()), new URI(_basePath + dstUri.getPath()));`
Original file line number	Diff line number	Diff line change
`@@ -109,6 +109,7 @@ public boolean doMove(URI srcUri, URI dstUri)`
`109`	`109`	`@Override`
`110`	`110`	`public boolean copyDir(URI srcUri, URI dstUri)`
`111`	`111`	`throws IOException {`
	`112`	`+ // TODO: support directory copy.`
`112`	`113`	`if (isDirectory(srcUri)) {`
`113`	`114`	`throw new UnsupportedOperationException("Azure FS doesn't support directory recursive copy!");`
`114`	`115`	`}`
Original file line number	Diff line number	Diff line change
`@@ -209,7 +209,7 @@ public void testFS()`
`209`	`209`	`try {`
`210`	`210`	`localPinotFS.copy(firstTempDir.toURI(), secondTempDir.toURI());`
`211`	`211`	`fail();`
`212`		`- } catch (UnsupportedOperationException e) {`
	`212`	`+ } catch (IllegalArgumentException e) {`
`213`	`213`	`// expected.`
`214`	`214`	`}`
`215`	`215`