luoyuxia commented on code in PR #21257:
URL: https://github.com/apache/flink/pull/21257#discussion_r1058898389
##########
flink-connectors/flink-connector-files/src/main/java/org/apache/flink/connector/file/table/PartitionLoader.java:
##########
@@ -148,23 +150,35 @@ private void overwrite(Path destDir) throws Exception {
}
/** Moves files from srcDir to destDir. */
- private void moveFiles(List<Path> srcDirs, Path destDir) throws Exception {
- for (Path srcDir : srcDirs) {
- if (!srcDir.equals(destDir)) {
- FileStatus[] srcFiles = listStatusWithoutHidden(fs, srcDir);
- if (srcFiles != null) {
- for (FileStatus srcFile : srcFiles) {
- Path srcPath = srcFile.getPath();
- Path destPath = new Path(destDir, srcPath.getName());
- // if it's not to move to local file system, just
rename it
- if (!isToLocal) {
- fs.rename(srcPath, destPath);
- } else {
- FileUtils.copy(srcPath, destPath, true);
+ private void moveFiles(List<Path> srcPaths, Path destDir, boolean
srcPathIsDir)
Review Comment:
We can check by `fs.getFileStatus(srcDir).isDir()`, but I prefer to still
keep the logic in here. The reason is as follows:
1: I think it's more clear to use method argument to make sure these srcpath
are files. Also, if we check the path is dir or not. Do we still need to check
the children path is dir or not?
2: It'll cost time to list file status
--
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: issues-unsubscr...@flink.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
