neils-dev commented on code in PR #4246: URL: https://github.com/apache/ozone/pull/4246#discussion_r1166160479
########## hadoop-ozone/ozonefs-common/src/main/java/org/apache/hadoop/fs/ozone/OzoneDelete.java: ########## @@ -0,0 +1,206 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * <p> + * http://www.apache.org/licenses/LICENSE-2.0 + * <p> + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.fs.ozone; + +import java.io.FileNotFoundException; +import java.io.IOException; +import java.net.URI; +import java.util.LinkedList; +import java.util.List; + +import org.apache.hadoop.classification.InterfaceAudience; +import org.apache.hadoop.classification.InterfaceStability; +import org.apache.hadoop.fs.ContentSummary; +import org.apache.hadoop.fs.Path; +import org.apache.hadoop.fs.PathIOException; +import org.apache.hadoop.fs.PathIsDirectoryException; +import org.apache.hadoop.fs.PathNotFoundException; +import org.apache.hadoop.fs.Trash; +import org.apache.hadoop.fs.shell.CommandFactory; +import org.apache.hadoop.fs.shell.CommandFormat; +import org.apache.hadoop.fs.shell.FsCommand; +import org.apache.hadoop.fs.shell.PathData; +import org.apache.hadoop.util.ToolRunner; +import static org.apache.hadoop.fs.CommonConfigurationKeysPublic.HADOOP_SHELL_SAFELY_DELETE_LIMIT_NUM_FILES; +import static org.apache.hadoop.fs.CommonConfigurationKeysPublic.HADOOP_SHELL_SAFELY_DELETE_LIMIT_NUM_FILES_DEFAULT; +import static org.apache.hadoop.ozone.OzoneConsts.OZONE_URI_DELIMITER; + +/** + * Classes that delete paths. + */ [email protected] [email protected] + +public final class OzoneDelete { + + private OzoneDelete() { + } + + public static void registerCommands(CommandFactory factory) { + factory.addClass(OzoneDelete.Rm.class, "-rm"); + factory.addClass(OzoneDelete.Rmr.class, "-rmr"); + } + + /** remove non-directory paths. */ + public static class Rm extends FsCommand { + public static final String NAME = "rm"; + public static final String USAGE = "[-f] [-r|-R] [-skipTrash] " + + "[-safely] <src> ..."; + public static final String DESCRIPTION = + "Delete all files that match the specified file pattern. " + + "Equivalent to the Unix command \"rm <src>\"\n" + + "-f: If the file does not exist, do not display a diagnostic " + + "message or modify the exit status to reflect an error.\n" + + "-[rR]: Recursively deletes directories.\n" + + "-skipTrash: option bypasses trash, if enabled, and immediately " + + "deletes <src>.\n" + + "-safely: option requires safety confirmation, if enabled, " + + "requires confirmation before deleting large directory with more " + + "than <hadoop.shell.delete.limit.num.files> files. Delay is " + + "expected when walking over large directory recursively to count " + + "the number of files to be deleted before the confirmation.\n"; + + private boolean skipTrash = false; + private boolean deleteDirs = false; + private boolean ignoreFNF = false; + private boolean safeDelete = false; + private boolean trailing = false; + + @Override + protected void processOptions(LinkedList<String> args) throws IOException { + CommandFormat cf = new CommandFormat( + 1, Integer.MAX_VALUE, "f", "r", "R", "skipTrash", "safely"); + cf.parse(args); + ignoreFNF = cf.getOpt("f"); + deleteDirs = cf.getOpt("r") || cf.getOpt("R"); + skipTrash = cf.getOpt("skipTrash"); + safeDelete = cf.getOpt("safely"); + } + + @Override + protected List<PathData> expandArgument(String arg) throws IOException { + try { + // handle trailing slash for symlinks + if (arg.endsWith(OZONE_URI_DELIMITER)) { + trailing = true; + } + return super.expandArgument(arg); + } catch (PathNotFoundException e) { + if (!ignoreFNF) { + throw e; + } + // prevent -f on a non-existent glob from failing + return new LinkedList<PathData>(); + } + } + + @Override + protected void processNonexistentPath(PathData item) throws IOException { + if (!ignoreFNF) { + super.processNonexistentPath(item); + } + } + + @Override + protected void processPath(PathData item) throws IOException { + boolean isSymlink = false; + if (item.fs.supportsSymlinks()) { + isSymlink = item.fs.getLinkTarget(item.path) != item.path; + } + // support posix rm of symlink in addition to rm -r for directories + if (item.stat.isDirectory() && !deleteDirs && !isSymlink) { + throw new PathIsDirectoryException(item.toString()); + } + // support posix symlink delete with trailing slash 'rm <path symlink>/' + // deletes contents of symlink bucket and retains symlink + Path path = item.path; + if (isSymlink && trailing) { + path = new Path(URI.create(path.toString() + OZONE_URI_DELIMITER)); + } + // TODO: if the user wants the trash to be used but there is any + // problem (ie. creating the trash dir, moving the item to be deleted, + // etc), then the path will just be deleted because moveToTrash returns + // false and it falls thru to fs.delete. this doesn't seem right + if (moveToTrash(item) || !canBeSafelyDeleted(item)) { + return; + } Review Comment: Looked into the moveToTrash() comment. > just return error to the user. So that they either need to deal with that trash issue first (maybe some ACL issue), or set -skipTrash. I suspect, like you that errors from creating the trash directory or renaming to trash will be handled properly with the exception thrown by trash - similar to ``` bash-4.2$ ozone fs -rm ofs://om/tmp/tmpfile rm: Failed to move to trash: ofs://om/tmp/tmpfile: User testuser2/[email protected] doesn't have DELETE permission to access key Volume:tmp Bucket:tmp Key:tmpfile. Consider using -skipTrash option ``` Here errors occurring from trash unable to create trash dir or renaming file are handled by exception handling and the file is not outright deleted by `fs.delete`. File remains intact leaving the user to resolve the issue through permission settings or `-skipTrash`. I'm looking to update the comment to reflect this. -- 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]
