You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@oozie.apache.org by Purshotam Shah <pu...@yahoo-inc.com> on 2014/11/19 22:47:00 UTC
Re: Review Request 24487: OOZIE-1913 Devise a way to turn off SLA
alerts for bundle/coordinator flexibly
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/24487/#review61210
-----------------------------------------------------------
Core logic looks great, few comments.
1. We are only supporting coord_name with bundle, it will be nice if we can support coord_id as well.
2. I guess we are not displaying sla alert information anywhere, it will be hard for user to know which action is disabled. I think it will be useful if we can display sla enable/disable information with some command and in Oozie UI SLA Tab.
client/src/main/java/org/apache/oozie/cli/OozieCLI.java
<https://reviews.apache.org/r/24487/#comment102732>
Has args should be true. SLA_DISABLE_ALERT need jobId.
client/src/main/java/org/apache/oozie/cli/OozieCLI.java
<https://reviews.apache.org/r/24487/#comment102733>
Has args should be true.
client/src/main/java/org/apache/oozie/cli/OozieCLI.java
<https://reviews.apache.org/r/24487/#comment102734>
Has args should be true.
client/src/main/java/org/apache/oozie/cli/OozieCLI.java
<https://reviews.apache.org/r/24487/#comment102735>
Is this used?
client/src/main/java/org/apache/oozie/cli/OozieCLI.java
<https://reviews.apache.org/r/24487/#comment102736>
Why optionsGroups? They should be added as option, like run, dryrun.
client/src/main/java/org/apache/oozie/client/OozieClient.java
<https://reviews.apache.org/r/24487/#comment102737>
Why not json responce, All oozie webservice responce is json.
client/src/main/java/org/apache/oozie/client/OozieClient.java
<https://reviews.apache.org/r/24487/#comment102740>
Not used anywhere.
core/src/main/java/org/apache/oozie/CoordinatorActionBean.java
<https://reviews.apache.org/r/24487/#comment104216>
Should include ignore status
core/src/main/java/org/apache/oozie/CoordinatorActionBean.java
<https://reviews.apache.org/r/24487/#comment104217>
Should include ignore status
core/src/main/java/org/apache/oozie/coord/CoordUtils.java
<https://reviews.apache.org/r/24487/#comment104226>
Will this work if job freqency is in different time zone?
core/src/main/java/org/apache/oozie/coord/CoordUtils.java
<https://reviews.apache.org/r/24487/#comment104220>
why name cabs?
core/src/main/java/org/apache/oozie/jms/JMSSLAEventListener.java
<https://reviews.apache.org/r/24487/#comment104199>
Why? I don't think it's being used anywhere.
core/src/main/java/org/apache/oozie/servlet/V0JobServlet.java
<https://reviews.apache.org/r/24487/#comment104201>
Not supported in v0
core/src/main/java/org/apache/oozie/servlet/V0JobServlet.java
<https://reviews.apache.org/r/24487/#comment104202>
Not supported in v0
core/src/main/java/org/apache/oozie/servlet/V0JobServlet.java
<https://reviews.apache.org/r/24487/#comment104203>
Not supported in v0
core/src/main/java/org/apache/oozie/servlet/V2JobServlet.java
<https://reviews.apache.org/r/24487/#comment104207>
This code used in three function, please move it to one sun-function().
core/src/main/java/org/apache/oozie/servlet/V2JobServlet.java
<https://reviews.apache.org/r/24487/#comment104251>
Since we have diffent command for different job_type, this logic can move to <jobType>XCommand.
core/src/main/java/org/apache/oozie/sla/BundleChangeSlaXCommand.java
<https://reviews.apache.org/r/24487/#comment104228>
BundleChangeSlaXCommand and CoordChangeSlaXCommand does same thing, in that case we can have only one class ChangeSlaXCommand.
core/src/main/java/org/apache/oozie/sla/BundleChangeSlaXCommand.java
<https://reviews.apache.org/r/24487/#comment102743>
Why requeue?
core/src/main/java/org/apache/oozie/sla/BundleDisableSlaAlertsXCommand.java
<https://reviews.apache.org/r/24487/#comment104230>
we can merge BundleDisableSlaAlertsXCommand and CoordDisableSlaAlertsXCommand to DisableSlaAlertsXCommand.
core/src/main/java/org/apache/oozie/sla/BundleDisableSlaAlertsXCommand.java
<https://reviews.apache.org/r/24487/#comment102744>
Do u need to load bundle bean? I guess we are not using it.
core/src/main/resources/oozie-default.xml
<https://reviews.apache.org/r/24487/#comment104231>
Is this used anywhere?
core/src/test/java/org/apache/oozie/command/coord/TestCoordSubmitXCommand.java
<https://reviews.apache.org/r/24487/#comment104232>
why sleep of 5 min. If you need it add it to specific testcase, it will slowdown the testcase execution.
- Purshotam Shah
On Oct. 20, 2014, 11:20 p.m., Mona Chitnis wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/24487/
> -----------------------------------------------------------
>
> (Updated Oct. 20, 2014, 11:20 p.m.)
>
>
> Review request for oozie.
>
>
> Bugs: OOZIE-1913
> https://issues.apache.org/jira/browse/OOZIE-1913
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> See Jira
>
>
> Diffs
> -----
>
> client/src/main/java/org/apache/oozie/cli/OozieCLI.java 9c2d14b
> client/src/main/java/org/apache/oozie/client/OozieClient.java 5e53a18
> client/src/main/java/org/apache/oozie/client/event/jms/JMSHeaderConstants.java 801ad7e
> client/src/main/java/org/apache/oozie/client/rest/RestConstants.java 4cc6606
> core/src/main/java/org/apache/oozie/CoordinatorActionBean.java 759e643
> core/src/main/java/org/apache/oozie/CoordinatorJobBean.java 2362084
> core/src/main/java/org/apache/oozie/command/SubmitTransitionXCommand.java 070cee5
> core/src/main/java/org/apache/oozie/command/bundle/BundleSubmitXCommand.java de78ab7
> core/src/main/java/org/apache/oozie/command/coord/CoordMaterializeTransitionXCommand.java 05b7a62
> core/src/main/java/org/apache/oozie/coord/CoordUtils.java 4643d73
> core/src/main/java/org/apache/oozie/executor/jpa/CoordActionQueryExecutor.java e6ab09b
> core/src/main/java/org/apache/oozie/executor/jpa/CoordJobQueryExecutor.java 4bccef4
> core/src/main/java/org/apache/oozie/jms/JMSSLAEventListener.java c19839f
> core/src/main/java/org/apache/oozie/service/CoordMaterializeTriggerService.java ee1085a
> core/src/main/java/org/apache/oozie/service/EventHandlerService.java 244c048
> core/src/main/java/org/apache/oozie/servlet/BaseJobServlet.java c94d1e2
> core/src/main/java/org/apache/oozie/servlet/SLAServlet.java 2578e41
> core/src/main/java/org/apache/oozie/servlet/V0JobServlet.java b160b46
> core/src/main/java/org/apache/oozie/servlet/V1JobServlet.java 8dc9608
> core/src/main/java/org/apache/oozie/servlet/V2JobServlet.java da81b49
> core/src/main/java/org/apache/oozie/sla/BundleChangeSlaXCommand.java PRE-CREATION
> core/src/main/java/org/apache/oozie/sla/BundleDisableSlaAlertsXCommand.java PRE-CREATION
> core/src/main/java/org/apache/oozie/sla/BundleEnableSlaAlertsXCommand.java PRE-CREATION
> core/src/main/java/org/apache/oozie/sla/CoordChangeSlaXCommand.java PRE-CREATION
> core/src/main/java/org/apache/oozie/sla/CoordDisableSlaAlertsXCommand.java PRE-CREATION
> core/src/main/java/org/apache/oozie/sla/CoordEnableSlaAlertsXCommand.java PRE-CREATION
> core/src/main/java/org/apache/oozie/sla/SLACalcStatus.java 189d5ea
> core/src/main/java/org/apache/oozie/sla/SLACalculator.java 20f93b5
> core/src/main/java/org/apache/oozie/sla/SLACalculatorMemory.java 188144e
> core/src/main/java/org/apache/oozie/sla/SLAOperations.java f5fc826
> core/src/main/java/org/apache/oozie/sla/service/SLAService.java 89615bc
> core/src/main/java/org/apache/oozie/util/CoordActionsInDateRange.java 7c2620c
> core/src/main/resources/oozie-default.xml 26eb7e0
> core/src/test/java/org/apache/oozie/command/coord/TestCoordSubmitXCommand.java f13e48f
> core/src/test/java/org/apache/oozie/coord/TestCoordUtils.java ae3f18d
> core/src/test/java/org/apache/oozie/jms/TestJMSSLAEventListener.java 30fd151
> core/src/test/java/org/apache/oozie/servlet/DagServletTestCase.java 48193c7
> core/src/test/java/org/apache/oozie/servlet/TestV2JobServlet.java fb203a6
> core/src/test/java/org/apache/oozie/sla/TestSLACalculatorMemory.java db3f6eb
> core/src/test/java/org/apache/oozie/store/TestCoordinatorStore.java b8b2405
>
> Diff: https://reviews.apache.org/r/24487/diff/
>
>
> Testing
> -------
>
> unit tests added, e-2-e test with CLI command done
>
>
> Thanks,
>
> Mona Chitnis
>
>