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



client/src/main/java/org/apache/oozie/client/OozieClient.java
<https://reviews.apache.org/r/31232/#comment119587>

    Can you rename this to FILTER_BUNDLE? so that other command can also use it.



core/src/main/java/org/apache/oozie/executor/jpa/sla/SLASummaryGetForFilterJPAExecutor.java
<https://reviews.apache.org/r/31232/#comment119615>

    Just a suggestion, this method is very big and complex to understand. Is 
that possible to break down in multiple functions?



core/src/main/java/org/apache/oozie/executor/jpa/sla/SLASummaryGetForFilterJPAExecutor.java
<https://reviews.apache.org/r/31232/#comment119606>

    I guess this should be                             .append(" AND 
s.expectedStartTS >= s.actualStartTS) ")



core/src/main/java/org/apache/oozie/executor/jpa/sla/SLASummaryGetForFilterJPAExecutor.java
<https://reviews.apache.org/r/31232/#comment119608>

    What if job is still running and it has missed duration. If job is running 
actualDuration will not be populated.



core/src/main/java/org/apache/oozie/executor/jpa/sla/SLASummaryGetForFilterJPAExecutor.java
<https://reviews.apache.org/r/31232/#comment119609>

    Can't we just use status. For end_miss and end_met, evenstatus will have 
correct status.



core/src/main/java/org/apache/oozie/servlet/V2SLAServlet.java
<https://reviews.apache.org/r/31232/#comment119610>

    We have already defined name pattern in BulkJPAExecutor. Can you please 
reuse that, in that way we can avoid duplicates.



core/src/main/java/org/apache/oozie/servlet/V2SLAServlet.java
<https://reviews.apache.org/r/31232/#comment119617>

    Why do we need this? Can't this be done in sql itsleft.
    for DURATION_MISS,
    id ( event_status=DURATION_MISS || ( actualDuration > expectedDuration))



core/src/main/java/org/apache/oozie/sla/SLASummaryBean.java
<https://reviews.apache.org/r/31232/#comment119618>

    I guess we have same logic for SLA UI to populate eventStatus. Can please 
resuse this code to avoid duplicate code.



core/src/test/java/org/apache/oozie/servlet/TestV2SLAServlet.java
<https://reviews.apache.org/r/31232/#comment119622>

    Can you also assert event_status?



core/src/test/java/org/apache/oozie/servlet/TestV2SLAServlet.java
<https://reviews.apache.org/r/31232/#comment119629>

    With  "event_status=DURATION_MISS;sla_status=IN_PROCESS,MET" option, you 
will should never get record with END_MISS.


- Purshotam Shah


On Feb. 20, 2015, 6:53 p.m., Ryota Egashira wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31232/
> -----------------------------------------------------------
> 
> (Updated Feb. 20, 2015, 6:53 p.m.)
> 
> 
> Review request for oozie.
> 
> 
> Repository: oozie-git
> 
> 
> Description
> -------
> 
> https://issues.apache.org/jira/browse/OOZIE-2146
> 
> 
> Diffs
> -----
> 
>   client/src/main/java/org/apache/oozie/client/OozieClient.java e4c93cd 
>   client/src/main/java/org/apache/oozie/client/rest/JsonTags.java b7cf0e7 
>   
> core/src/main/java/org/apache/oozie/executor/jpa/sla/SLASummaryGetForFilterJPAExecutor.java
>  b55cda3 
>   core/src/main/java/org/apache/oozie/servlet/V2SLAServlet.java a0fe1b6 
>   core/src/main/java/org/apache/oozie/sla/SLASummaryBean.java 9907dd0 
>   core/src/test/java/org/apache/oozie/servlet/DagServletTestCase.java 48193c7 
>   core/src/test/java/org/apache/oozie/servlet/TestV2SLAServlet.java 5f51b22 
>   docs/src/site/twiki/DG_SLAMonitoring.twiki acf8ac1 
> 
> Diff: https://reviews.apache.org/r/31232/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Ryota Egashira
> 
>

Reply via email to