----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41587/#review111441 -----------------------------------------------------------
client/src/main/java/org/apache/falcon/client/FalconCLIException.java (line 80) <https://reviews.apache.org/r/41587/#comment171635> Please add commen on other versions of this method to indicate others to use this method. client/src/main/java/org/apache/falcon/client/FalconCLIException.java (line 81) <https://reviews.apache.org/r/41587/#comment171626> Ideally, there should be only one way to do it but I assume you are not changing old methods and just adding new one for this API, to limit the scope of the change. client/src/main/java/org/apache/falcon/client/FalconCLIException.java (line 84) <https://reviews.apache.org/r/41587/#comment171625> If you are planning to migrate in phases, then it will be nice to add a comment on this method to not be used and developers should use the method above it? client/src/main/java/org/apache/falcon/client/FalconCLIException.java (line 88) <https://reviews.apache.org/r/41587/#comment171634> I suspect status is already checked before calling this method. client/src/main/java/org/apache/falcon/client/FalconCLIException.java (line 93) <https://reviews.apache.org/r/41587/#comment171628> Just curious, is it possible to decode the class to be used from status code? client/src/main/java/org/apache/falcon/client/FalconCLIException.java (line 109) <https://reviews.apache.org/r/41587/#comment171627> Incorrect indentation? client/src/main/java/org/apache/falcon/client/FalconClient.java (line 956) <https://reviews.apache.org/r/41587/#comment171629> You need to add null checks for query params. client/src/main/java/org/apache/falcon/client/FalconClient.java (line 958) <https://reviews.apache.org/r/41587/#comment171630> You need to add the "end" parameter as well after checking for null. client/src/main/java/org/apache/falcon/client/FalconClient.java (line 1258) <https://reviews.apache.org/r/41587/#comment171631> Please add a comment on the other version of this method to point users to use this method with appropriate reasons. prism/src/main/java/org/apache/falcon/FalconWebException.java (line 108) <https://reviews.apache.org/r/41587/#comment171632> I think we should delete other class specific versions and consolidate exception creation. prism/src/main/java/org/apache/falcon/resource/AbstractInstanceManager.java (line 520) <https://reviews.apache.org/r/41587/#comment171633> Please log the entityType which was passed along with the message. - Ajay Yadava On Dec. 19, 2015, 9:19 a.m., Praveen Adlakha wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/41587/ > ----------------------------------------------------------- > > (Updated Dec. 19, 2015, 9:19 a.m.) > > > Review request for Falcon. > > > Bugs: falcon-1565 > https://issues.apache.org/jira/browse/falcon-1565 > > > Repository: falcon-git > > > Description > ------- > > Listing API non-intuitive response if time > endTime > > > Diffs > ----- > > client/src/main/java/org/apache/falcon/cli/FalconInstanceCLI.java 6ea34da > client/src/main/java/org/apache/falcon/client/FalconCLIException.java > 51ef952 > client/src/main/java/org/apache/falcon/client/FalconClient.java b4b1762 > prism/src/main/java/org/apache/falcon/FalconWebException.java 7324997 > prism/src/main/java/org/apache/falcon/resource/AbstractInstanceManager.java > da2fea7 > prism/src/main/java/org/apache/falcon/resource/channel/HTTPChannel.java > 6801398 > > prism/src/main/java/org/apache/falcon/resource/proxy/InstanceManagerProxy.java > 96c99f0 > > Diff: https://reviews.apache.org/r/41587/diff/ > > > Testing > ------- > > yes > > > Thanks, > > Praveen Adlakha > >
