----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41587/#review111437 -----------------------------------------------------------
client/src/main/java/org/apache/falcon/cli/FalconInstanceCLI.java <https://reviews.apache.org/r/41587/#comment171610> Why is doAsUser removed? client/src/main/java/org/apache/falcon/client/FalconCLIException.java (line 80) <https://reviews.apache.org/r/41587/#comment171616> Can't we replace the existing methods with these, rather than adding new ones? client/src/main/java/org/apache/falcon/client/FalconClient.java (line 949) <https://reviews.apache.org/r/41587/#comment171611> Let us not introduce a new method. I believe you introduced this in order to supply FeedInstanceResult to appropriate methods. It would be better to refactor the code to supply the appropriate class in order methods too. client/src/main/java/org/apache/falcon/client/FalconClient.java (line 955) <https://reviews.apache.org/r/41587/#comment171612> nit : Better not leave commented code behind. prism/src/main/java/org/apache/falcon/resource/AbstractInstanceManager.java (line 530) <https://reviews.apache.org/r/41587/#comment171618> Why remove logging? prism/src/main/java/org/apache/falcon/resource/AbstractInstanceManager.java <https://reviews.apache.org/r/41587/#comment171619> Why not modify the newInstanceException method rather than use a generic newAPIException method? prism/src/main/java/org/apache/falcon/resource/AbstractInstanceManager.java (line 879) <https://reviews.apache.org/r/41587/#comment171621> Generally, the web exception is thrown by the JAX-RS layer, rather than the resource layer. prism/src/main/java/org/apache/falcon/resource/channel/HTTPChannel.java (line 36) <https://reviews.apache.org/r/41587/#comment171613> This will result in checkstyle errors. Expand the imports, please. Guess you'll need to change your IDE preferencens to not automatically collapse. prism/src/main/java/org/apache/falcon/resource/channel/HTTPChannel.java (line 48) <https://reviews.apache.org/r/41587/#comment171615> whitespace / format differences with the code base and your IDE prefs. prism/src/main/java/org/apache/falcon/resource/channel/HTTPChannel.java (line 120) <https://reviews.apache.org/r/41587/#comment171622> Generally, the web exception is thrown by the JAX-RS layer, rather than the resource layer. prism/src/main/java/org/apache/falcon/resource/proxy/InstanceManagerProxy.java (line 622) <https://reviews.apache.org/r/41587/#comment171623> the catch clause for Throwable is already handling all exceptions. Better to change there? - Pallavi Rao 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 > >
