falcon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Ajay Yadava" <ajayn...@gmail.com>
Subject Re: Review Request 41587: Listing API non-intuitive response if time > endTime
Date Mon, 21 Dec 2015 06:25:33 GMT

-----------------------------------------------------------
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
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message