okumin commented on code in PR #6006:
URL: https://github.com/apache/hive/pull/6006#discussion_r2383929152
##########
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/FileUtils.java:
##########
@@ -114,12 +92,30 @@ public static boolean moveToTrash(FileSystem fs, Path f,
Configuration conf, boo
.contains("Snapshot"))
deleteReplRelatedSnapshots(fs, f);
// retry delete after attempting to delete replication related snapshots
- result = fs.delete(f, true);
+ fs.delete(f, true);
}
- if (!result) {
- LOG.error("Failed to delete " + f);
+ return true;
+ }
+
+ /**
+ * Move a particular file or directory to the trash.
+ * @param fs FileSystem to use
+ * @param f path of file or directory to move to trash.
+ * @param conf configuration object
+ * @return true if move successful
+ */
+ public static boolean moveToTrash(FileSystem fs, Path f, Configuration conf)
{
Review Comment:
This can be private
##########
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/FileUtils.java:
##########
@@ -114,12 +92,30 @@ public static boolean moveToTrash(FileSystem fs, Path f,
Configuration conf, boo
.contains("Snapshot"))
deleteReplRelatedSnapshots(fs, f);
// retry delete after attempting to delete replication related snapshots
- result = fs.delete(f, true);
+ fs.delete(f, true);
}
- if (!result) {
- LOG.error("Failed to delete " + f);
+ return true;
+ }
+
+ /**
+ * Move a particular file or directory to the trash.
+ * @param fs FileSystem to use
+ * @param f path of file or directory to move to trash.
+ * @param conf configuration object
+ * @return true if move successful
+ */
+ public static boolean moveToTrash(FileSystem fs, Path f, Configuration conf)
{
+ LOG.debug("moving " + f + " to trash");
Review Comment:
Probably, you will see warning messages from SonarQube about how to use
slf4j once you rebase this branch
##########
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/FileUtils.java:
##########
@@ -114,12 +92,30 @@ public static boolean moveToTrash(FileSystem fs, Path f,
Configuration conf, boo
.contains("Snapshot"))
deleteReplRelatedSnapshots(fs, f);
// retry delete after attempting to delete replication related snapshots
- result = fs.delete(f, true);
+ fs.delete(f, true);
}
- if (!result) {
- LOG.error("Failed to delete " + f);
+ return true;
Review Comment:
This method is likely to always return true. Is it expected?
##########
standalone-metastore/metastore-common/src/main/java/org/apache/hadoop/hive/metastore/utils/FileUtils.java:
##########
@@ -71,41 +71,19 @@ public boolean accept(Path p) {
}
};
- /**
- * Move a particular file or directory to the trash.
- * @param fs FileSystem to use
- * @param f path of file or directory to move to trash.
- * @param conf configuration object
- * @return true if move successful
- * @throws IOException
- */
- public static boolean moveToTrash(FileSystem fs, Path f, Configuration conf,
boolean purge)
+ public static boolean deleteDir(FileSystem fs, Path f, boolean ifPurge,
Configuration conf)
throws IOException {
- LOG.debug("deleting " + f);
- boolean result;
+ if (!fs.exists(f)) {
+ LOG.warn("The path to delete does not exist: " + f);
+ return true;
+ }
+ if (!ifPurge && moveToTrash(fs, f, conf)) {
+ return true;
+ }
try {
- if (!fs.exists(f)) {
- LOG.warn("The path to moveToTrash does not exist: " + f);
- return true;
- }
- if (purge) {
- LOG.debug("purge is set to true. Not moving to Trash " + f);
- } else {
- result = Trash.moveToAppropriateTrash(fs, f, conf);
- if (result) {
- LOG.trace("Moved to trash: " + f);
- return true;
- }
- }
- } catch (IOException ioe) {
// for whatever failure reason including that trash has lower encryption
zone
// retry with force delete
Review Comment:
I guess the current logic can't reproduce this behavior, I'm not sure that
it is really necessary
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
