sunithabeeram commented on a change in pull request #3834: Unify move method in
PinotFS
URL: https://github.com/apache/incubator-pinot/pull/3834#discussion_r260525878
##########
File path:
pinot-filesystem/src/main/java/org/apache/pinot/filesystem/PinotFS.java
##########
@@ -58,31 +69,54 @@ public abstract boolean delete(URI segmentUri, boolean
forceDelete)
/**
* Moves the file or directory from the src to dst. Does not keep the
original file. If the dst has parent directories
* that haven't been created, this method will create all the necessary
parent directories.
- * If both src and dst are files, dst will be overwritten.
- * If src is a file and dst is a directory, src file will get moved under
dst directory.
- * If both src and dst are directories, src directory will get moved under
dst directory.
- * If src is a directory and dst is a file, operation will fail.
+ * Note: In Pinot we recommend the full paths of both src and dst be
specified.
* For example, if a file /a/b/c is moved to a file /x/y/z, in the case of
overwrite, the directory /a/b still exists,
* but will not contain the file 'c'. Instead, /x/y/z will contain the
contents of 'c'.
- * If a file /a is moved to a directory /x/y, all the original files under
/x/y will be kept.
+ * If src is a directory /a/b which contains two files /a/b/c and /a/b/d,
and the dst is /x/y, the result would be
+ * that the directory /a/b under /a gets removed and dst directory contains
two files which is /x/y/c and /x/y/d.
+ * If src is a directory /a/b needs to be moved under another directory
/x/y, please specify the dst to /x/y/b.
* @param srcUri URI of the original file
* @param dstUri URI of the final file location
* @param overwrite true if we want to overwrite the dstURI, false otherwise
* @return true if move is successful
* @throws IOException on IO failure
*/
- public abstract boolean move(URI srcUri, URI dstUri, boolean overwrite)
+ public boolean move(URI srcUri, URI dstUri, boolean overwrite)
+ throws IOException {
+ if (!exists(srcUri)) {
+ LOGGER.warn("Source {} does not exist", srcUri);
+ return false;
+ }
+ if (exists(dstUri)) {
+ if (overwrite) {
+ delete(dstUri, true);
+ } else {
+ // dst file exists, returning
+ LOGGER.warn("Cannot move {} to {}. Destination exists and overwrite
flag set to false.", srcUri, dstUri);
+ return false;
+ }
+ } else {
+ // ensures the parent path of dst exists.
+ URI parentUri = Paths.get(dstUri).getParent().toUri();
+ mkdir(parentUri);
+ }
+ return doMove(srcUri, dstUri);
+ }
+
+ /**
+ * Does the actual behavior of move in each FS.
+ */
+ protected abstract boolean doMove(URI srcUri, URI dstUri)
throws IOException;
/**
* Copies the file or directory from the src to dst. The original file is
retained. If the dst has parent directories
* that haven't been created, this method will create all the necessary
parent directories.
- * If both src and dst are files, dst will be overwritten.
- * If src is a file and dst is a directory, src file will get copied under
dst directory.
- * If both src and dst are directories, src directory will get copied under
dst directory.
- * If src is a directory and dst is a file, operation will fail.
- * For example, if a file /x/y/z is copied to /a/b/c, /x/y/z will be
retained and /x/y/z will also be present as /a/b/c;
- * if a file /a is copied to a directory /x/y, all the original files under
/x/y will be kept.
+ * Note: In Pinot we recommend the full paths of both src and dst be
specified.
+ * For example, if a file /a/b/c is copied to a file /x/y/z, in the case of
overwrite, the directory /a/b still exists,
Review comment:
Lets not have incorrect documentation though, please :)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
