-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3043/#review3692
-----------------------------------------------------------



trunk/core/src/main/java/org/apache/oozie/command/coord/CoordActionInputCheckXCommand.java
<https://reviews.apache.org/r/3043/#comment8217>

    is there any reason you re-order the two statement. 
    it seems logging the message doesn't depend on checking input.  Are you 
intending to add the status to the log message



trunk/core/src/main/java/org/apache/oozie/command/coord/CoordActionInputCheckXCommand.java
<https://reviews.apache.org/r/3043/#comment8216>

    it seems the for loop is overloaded here and makes the code not very 
readable. You may iterate through the list and check for the existence of the 
path during the iteration and do right action accordingly
    
    As well, the variable allExists is not right as it given the impression you 
are checking for the existence of all the paths which is not correct. 
    
    I believe the code need to be re-written on a more simpler and readable way.
    
    as well based on this implementation, you only returning the status of the 
last path. so if all the previous paths exist but the last one in non-exist. 
you will return false.  



trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordActionInputCheckXCommand.java
<https://reviews.apache.org/r/3043/#comment8218>

    overloading the index of different meanings looks confusing. you may get 
red of the index and use actualMissDeps in the if statement



trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordActionInputCheckXCommand.java
<https://reviews.apache.org/r/3043/#comment8219>

    which dependency ... you may include the missing dependency in the error 
message
    



trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordActionInputCheckXCommand.java
<https://reviews.apache.org/r/3043/#comment8220>

    same as before



trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordActionInputCheckXCommand.java
<https://reviews.apache.org/r/3043/#comment8221>

    same as before
    



trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordActionInputCheckXCommand.java
<https://reviews.apache.org/r/3043/#comment8222>

    same as before. spill out the missed dependency


- Mohamed


On 2011-12-07 02:05:15, Mona Chitnis wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3043/
> -----------------------------------------------------------
> 
> (Updated 2011-12-07 02:05:15)
> 
> 
> Review request for oozie, Mohammad Islam and Angelo K. Huang.
> 
> 
> Summary
> -------
> 
> The "Missing Dependencies" for any coordinator action lists all the 
> dependencies after first dependency is missing. Even though the subsequent 
> dependencies are present. This patch is a small correction to ensure only the 
> data dependencies which are not ready/resolved are added to the "Missing 
> Dependencies" list.
> 
> 
> This addresses bug OOZIE-563.
>     https://issues.apache.org/jira/browse/OOZIE-563
> 
> 
> Diffs
> -----
> 
>   
> trunk/core/src/test/java/org/apache/oozie/command/coord/TestCoordActionInputCheckXCommand.java
>  1211247 
>   
> trunk/core/src/main/java/org/apache/oozie/command/coord/CoordActionInputCheckXCommand.java
>  1211247 
> 
> Diff: https://reviews.apache.org/r/3043/diff
> 
> 
> Testing
> -------
> 
> yes
> 
> 
> Thanks,
> 
> Mona
> 
>

Reply via email to