walterddr commented on a change in pull request #8162:
URL: https://github.com/apache/pinot/pull/8162#discussion_r803941201
##########
File path:
pinot-spi/src/main/java/org/apache/pinot/spi/filesystem/LocalPinotFS.java
##########
@@ -85,7 +85,7 @@ public boolean doMove(URI srcUri, URI dstUri)
@Override
public boolean copy(URI srcUri, URI dstUri)
throws IOException {
- copy(toFile(srcUri), toFile(dstUri));
+ copy(toFile(srcUri), toFile(dstUri), false);
Review comment:
the fact is that after this PR
LocalPinotFS.copy(srcUri, dstUri) throws exception if srcUri is a directory.
S3PinotFS.copy(srcUri, dstUri) return a recursive copy if srcUri is a
directory.
this basically means users who needs to do a recursive copying would need to
know exactly what underlying implementation of the PinotFS it has. i am not
sure that's the right abstraction for this API.
my suggestion is to
1. make the current S3PintoFS.copy(srcUri, dstUri) to also throw exception
when the URI is a folder. thus make it consistent with LocalFS
2. move the current recursive copy implementation in S3 to a new API:
PinotFS.copy(srcUri, dstUri, recursive = true).
--
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: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
