Matthew Jacobs has posted comments on this change. Change subject: IMPALA-3230: Spurious errors from DeleteOldLogs() ......................................................................
Patch Set 2: Regarding our in person conversation about testing, I thought it'd be weird to return the number of logs deleted in order to test it, and I think that's kinda ugly, but just thought of this which seems simple and good enough for testing: DeleteOldLogs function returns a Status. That would be OK in the normal path or when this returns this specific error code. In other cases we can return an error Status with the message you log now, and the caller can check if !ok and simply log the error status message. Then a test case can just make sure that a path which doesn't match any files returns an OK status. -- To view, visit http://gerrit.cloudera.org:8080/2596 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I485b7c97b01bf17ad75cf8a456b2e4fbce91d31d Gerrit-PatchSet: 2 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Thomas Tauber-Marshall <[email protected]> Gerrit-Reviewer: Dan Hecht <[email protected]> Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]> Gerrit-HasComments: No
