Alex Behm has posted comments on this change. Change subject: IMPALA-3949: Log the error message in FileSystemUtil.copyToLocal() ......................................................................
Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/4125/4/fe/src/main/java/com/cloudera/impala/common/FileSystemUtil.java File fe/src/main/java/com/cloudera/impala/common/FileSystemUtil.java: Line 379: public static boolean copyToLocal(Path source, Path dest, StringBuilder errorTrace) { This pattern of returning a boolean and populating an error msg seems pretty weird to me. Isn't that what exceptions are for? Seems easier to me to just throw the exception and ditch the boolean return value. There are only a few callers of these functions. -- To view, visit http://gerrit.cloudera.org:8080/4125 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I5664a75aa837951de1d5dcc90e43bd8f313736b8 Gerrit-PatchSet: 4 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Bharath Vissapragada <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Bharath Vissapragada <[email protected]> Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-HasComments: Yes
