adoroszlai commented on code in PR #7864:
URL: https://github.com/apache/ozone/pull/7864#discussion_r1953202912
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/cli/GenericCli.java:
##########
@@ -123,4 +128,20 @@ protected PrintWriter out() {
protected PrintWriter err() {
return cmd.getErr();
}
+
+ public static String handleFileSystemException(FileSystemException e, File
dataFile) {
+ String message = e.getMessage();
+ String errorMessage;
+ if (e instanceof NoSuchFileException) {
+ errorMessage = String.format("Error: File not found: %s",
dataFile.getPath());
+ } else if (e instanceof AccessDeniedException) {
+ errorMessage = String.format("Error: Access denied to file: %s",
dataFile.getPath());
+ } else if (e instanceof FileAlreadyExistsException) {
+ errorMessage = String.format("Error: File already exists: %s",
dataFile.getPath());
+ } else {
+ errorMessage = String.format("Error with file: %s. Details: %s",
dataFile.getPath(), message);
+ }
Review Comment:
The parameter `dataFile` is not needed, `FileSystemException` has the path.
`FileSystemException` may have one or more of the following:
- file
- otherFile
- reason
All of these, if present, are added to the message. We need to
conditionally add the description ("File not found", etc.), only if
`e.getReason() == null`.
Otherwise, if reason is set, we can simply use the message from the
exception as is.
`handleFileSystemException` should be only called from `printError`, if the
error is indeed a `FileSystemException`. `printError` is used as a generic
exception handler. This way `FileSystemException` will be handled in all
commands, not just `ozone sh key put`. And we don't even need to add `catch`
all over the codebase.
##########
hadoop-ozone/cli-shell/src/main/java/org/apache/hadoop/ozone/shell/keys/PutKeyHandler.java:
##########
@@ -128,6 +128,8 @@ private void async(
try (InputStream input = Files.newInputStream(dataFile.toPath());
OutputStream output = createOrReplaceKey(bucket, keyName,
dataFile.length(), keyMetadata, replicationConfig)) {
IOUtils.copyBytes(input, output, chunkSize);
+ } catch (FileSystemException e) {
+ out().println(GenericCli.handleFileSystemException(e, dataFile));
Review Comment:
No need to catch it.
##########
hadoop-ozone/cli-shell/src/main/java/org/apache/hadoop/ozone/shell/keys/PutKeyHandler.java:
##########
@@ -82,14 +83,13 @@ protected void execute(OzoneClient client, OzoneAddress
address)
String keyName = address.getKeyName();
File dataFile = new File(fileName);
- if (!dataFile.exists()) {
- throw new FileNotFoundException("Error: File not found: " + fileName);
- }
if (isVerbose()) {
try (InputStream stream = Files.newInputStream(dataFile.toPath())) {
String hash = DigestUtils.sha256Hex(stream);
out().printf("File sha256 checksum : %s%n", hash);
+ } catch (FileSystemException e) {
+ out().println(GenericCli.handleFileSystemException(e, dataFile));
Review Comment:
No need to catch the exception here.
##########
hadoop-ozone/dist/src/main/smoketest/basic/ozone-shell-lib.robot:
##########
@@ -131,8 +131,7 @@ Test ozone shell errors
${result} = Execute and checkrc ozone sh bucket create
${protocol}${server}/${volume}/bucket1 255
Should contain ${result} QUOTA_ERROR
Execute and checkrc ozone sh volume delete
${protocol}${server}/${volume} 0
- ${result} = Execute and checkrc ozone sh key put
${protocol}${server}/${volume}/bucket1/key1 sample.txt 255
- Should Match Regexp ${result}
Error: File not found: .*
Review Comment:
Please keep the test case.
##########
hadoop-ozone/cli-shell/src/main/java/org/apache/hadoop/ozone/shell/keys/PutKeyHandler.java:
##########
@@ -173,6 +175,8 @@ private void stream(
off += writeLen;
len -= writeLen;
}
+ } catch (FileSystemException e) {
+ out().println(GenericCli.handleFileSystemException(e, dataFile));
Review Comment:
Not needed.
--
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...@ozone.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@ozone.apache.org
For additional commands, e-mail: issues-h...@ozone.apache.org
