sunithabeeram commented on a change in pull request #3834: Unify move method in
PinotFS
URL: https://github.com/apache/incubator-pinot/pull/3834#discussion_r260430406
##########
File path:
pinot-filesystem/src/main/java/org/apache/pinot/filesystem/PinotFS.java
##########
@@ -58,31 +63,53 @@ 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 a directory /a/b/ is renamed to another directory /x/y/, the directory
/x/y/ will contains the content of /a/b/.
+ * If a directory /a/b/ is moved under the directory /x/y/, the dst needs to
be specify as /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,
+ * but will not contain the file 'c'. Instead, /x/y/z will contain the
contents of 'c'.
Review comment:
That seems wrong. Source will still have c. Its just that if /x/y/z had a c,
c will be overwritten.
----------------------------------------------------------------
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]
