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



Please update https://issues.apache.org/jira/browse/OOZIE-2509 with some 
details on the issue and its cause and summary of changes in the patch. Anyone 
other than me cannot get any context about the issue or what the patch does.

First pass - Review for just test case changes. Still going through code 
refactor.


core/src/main/java/org/apache/oozie/command/sla/SLACoordActionJobEventXCommand.java
 (line 48)
<https://reviews.apache.org/r/46390/#comment193065>

    Use CoordActionQueryExecutor with GET_COORD_ACTION. We also have to change 
to load only required fields. Create a new jira for that.



core/src/main/java/org/apache/oozie/command/sla/SLACoordActionJobEventXCommand.java
 (line 63)
<https://reviews.apache.org/r/46390/#comment193066>

    updateSLAInformation



core/src/main/java/org/apache/oozie/command/sla/SLAJobEventXCommand.java (line 
41)
<https://reviews.apache.org/r/46390/#comment193071>

    isReQueueRequired should be implemented and return false



core/src/main/java/org/apache/oozie/command/sla/SLAJobEventXCommand.java (line 
42)
<https://reviews.apache.org/r/46390/#comment193074>

    Remove and use *QueryExecutor classes



core/src/main/java/org/apache/oozie/command/sla/SLAJobEventXCommand.java (line 
69)
<https://reviews.apache.org/r/46390/#comment193070>

    Why 0? Why not leave it at default?



core/src/test/java/org/apache/oozie/service/TestHASLAService.java (lines 361 - 
364)
<https://reviews.apache.org/r/46390/#comment193072>

    Remove dead code



core/src/test/java/org/apache/oozie/sla/TestSLACalculatorMemory.java (line 471)
<https://reviews.apache.org/r/46390/#comment193073>

    Should be MISS as duration is missed



core/src/test/java/org/apache/oozie/sla/TestSLACalculatorMemory.java (line 703)
<https://reviews.apache.org/r/46390/#comment193076>

    Whitespaces will become -1 in pre-commit build. Present in other classes as 
well



core/src/test/java/org/apache/oozie/sla/TestSLACalculatorMemory.java (line 886)
<https://reviews.apache.org/r/46390/#comment193078>

    Please just use static String constants for RUNNING and SUCCEEDED statuses 
instead of doing toString() on enum everytime. 
    
    Also better practice to use name() instead of toString() with enums.



core/src/test/java/org/apache/oozie/sla/TestSLAJobEventListener.java (line 107)
<https://reviews.apache.org/r/46390/#comment193080>

    Why set expected end and change to end_miss? We need a test for start_miss



core/src/test/java/org/apache/oozie/sla/TestSLAService.java 
<https://reviews.apache.org/r/46390/#comment193083>

    Why remove this?



core/src/test/java/org/apache/oozie/sla/TestSLAService.java (line 229)
<https://reviews.apache.org/r/46390/#comment193086>

    Why change WAITING to FAILED? Need a test case for waiting.


- Rohini Palaniswamy


On April 19, 2016, 8:54 p.m., Purshotam Shah wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/46390/
> -----------------------------------------------------------
> 
> (Updated April 19, 2016, 8:54 p.m.)
> 
> 
> Review request for oozie.
> 
> 
> Bugs: OOZIE-2509
>     https://issues.apache.org/jira/browse/OOZIE-2509
> 
> 
> Repository: oozie-git
> 
> 
> Description
> -------
> 
> SLA job status can stuck in running state
> 
> 
> Diffs
> -----
> 
>   
> core/src/main/java/org/apache/oozie/command/sla/SLACoordActionJobEventXCommand.java
>  e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
>   
> core/src/main/java/org/apache/oozie/command/sla/SLACoordActionJobHistoryXCommand.java
>  e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
>   core/src/main/java/org/apache/oozie/command/sla/SLAJobEventXCommand.java 
> e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
>   core/src/main/java/org/apache/oozie/command/sla/SLAJobHistoryXCommand.java 
> e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
>   
> core/src/main/java/org/apache/oozie/command/sla/SLAWorkflowActionJobEventXCommand.java
>  e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
>   
> core/src/main/java/org/apache/oozie/command/sla/SLAWorkflowActionJobHistoryXCommand.java
>  e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
>   
> core/src/main/java/org/apache/oozie/command/sla/SLAWorkflowJobEventXCommand.java
>  e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
>   
> core/src/main/java/org/apache/oozie/command/sla/SLAWorkflowJobHistoryXCommand.java
>  e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
>   core/src/main/java/org/apache/oozie/sla/SLACalculatorMemory.java 
> 42313fd3ab804c527afc115402295916b29f63d2 
>   core/src/main/java/org/apache/oozie/sla/SLAXCommandFactory.java 
> e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 
>   core/src/test/java/org/apache/oozie/service/TestHASLAService.java 
> 795db37316d6ced825618f6976133691c02bd940 
>   core/src/test/java/org/apache/oozie/sla/TestSLACalculatorMemory.java 
> 432efef6989951b0d66405dd3762cbfe2c965556 
>   core/src/test/java/org/apache/oozie/sla/TestSLAEventGeneration.java 
> 7a710c28cf035e528e7d024f254205d6c52d3309 
>   core/src/test/java/org/apache/oozie/sla/TestSLAJobEventListener.java 
> ebb12f77552f26aba962dddc63e3158d007b07a4 
>   core/src/test/java/org/apache/oozie/sla/TestSLAService.java 
> c3bc110d825f4e3b9a1afe1becd4b045e8c6dacf 
> 
> Diff: https://reviews.apache.org/r/46390/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Purshotam Shah
> 
>

Reply via email to