impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alex Behm (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-3949: Log the error message in FileSystemUtil.copyToLocal()
Date Fri, 26 Aug 2016 18:25:10 GMT
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 <bharathv@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Bharath Vissapragada <bharathv@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message