[
https://issues.apache.org/jira/browse/OOZIE-677?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13203057#comment-13203057
]
[email protected] commented on OOZIE-677:
-----------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3756/#review4883
-----------------------------------------------------------
Comments on consistency in coding, more helpful error messages and test
coverage.
trunk/client/src/main/java/org/apache/oozie/cli/OozieCLI.java
<https://reviews.apache.org/r/3756/#comment10784>
The message is very terse without any information on the ability to specify
a list. I noticed that the twiki has the details. I am expecting few people to
refer to the twiki and more people will refer to the command line message. Can
we improve on the message here?
trunk/client/src/main/java/org/apache/oozie/client/OozieClient.java
<https://reviews.apache.org/r/3756/#comment10757>
Shouldn't the list of arguments be (jobId, null, 0, 0) ?
trunk/client/src/main/java/org/apache/oozie/client/OozieClient.java
<https://reviews.apache.org/r/3756/#comment10758>
Shouldn't this be (jobId, filter, start, len) ?
trunk/core/src/main/java/org/apache/oozie/BundleEngine.java
<https://reviews.apache.org/r/3756/#comment10759>
Why are you using filterStr instead of filter - its not consistent with
your usage in other methods.
trunk/core/src/main/java/org/apache/oozie/CoordinatorEngine.java
<https://reviews.apache.org/r/3756/#comment10760>
Why are you using filterStr instead of filter - its not consistent with
your usage in other methods.
trunk/core/src/main/java/org/apache/oozie/CoordinatorEngine.java
<https://reviews.apache.org/r/3756/#comment10761>
Can you make this filterList - between filterStr and filter - there is no
way to figure out the data types
trunk/core/src/main/java/org/apache/oozie/CoordinatorEngine.java
<https://reviews.apache.org/r/3756/#comment10774>
Can you add detailed comments on the expectation of this method - there are
several assumptions here in the code that need to be called out.
trunk/core/src/main/java/org/apache/oozie/CoordinatorEngine.java
<https://reviews.apache.org/r/3756/#comment10762>
Can you include the pair to help the user with the remedial measure?
trunk/core/src/main/java/org/apache/oozie/CoordinatorEngine.java
<https://reviews.apache.org/r/3756/#comment10764>
What are the valid values?
trunk/core/src/main/java/org/apache/oozie/CoordinatorEngine.java
<https://reviews.apache.org/r/3756/#comment10765>
Is status the only filter?
trunk/core/src/main/java/org/apache/oozie/CoordinatorEngine.java
<https://reviews.apache.org/r/3756/#comment10766>
What are the valid filter keys?
trunk/core/src/main/java/org/apache/oozie/CoordinatorEngine.java
<https://reviews.apache.org/r/3756/#comment10767>
What happens if the token does not contain "=" ? The error handling is not
robust
trunk/core/src/main/java/org/apache/oozie/command/coord/CoordJobXCommand.java
<https://reviews.apache.org/r/3756/#comment10770>
Can you change this to filterList?
trunk/core/src/main/java/org/apache/oozie/command/coord/CoordJobXCommand.java
<https://reviews.apache.org/r/3756/#comment10771>
Can you change this to filterList?
trunk/core/src/main/java/org/apache/oozie/executor/jpa/CoordJobGetActionsSubsetJPAExecutor.java
<https://reviews.apache.org/r/3756/#comment10773>
Can you change this to filterList?
trunk/core/src/main/java/org/apache/oozie/executor/jpa/CoordJobGetActionsSubsetJPAExecutor.java
<https://reviews.apache.org/r/3756/#comment10772>
Can you change this to filterList?
trunk/core/src/main/java/org/apache/oozie/executor/jpa/CoordJobGetActionsSubsetJPAExecutor.java
<https://reviews.apache.org/r/3756/#comment10778>
Add a brief note about the method and change the name to getStatusClause or
something more meaningful
trunk/core/src/test/java/org/apache/oozie/TestCoordinatorEngine.java
<https://reviews.apache.org/r/3756/#comment10781>
Can you add a check for the following too:
1. empty filter list (if possible)
2. filter list with the '=' missing
3. filter list with nothing after '='
trunk/core/src/test/java/org/apache/oozie/TestCoordinatorEngine.java
<https://reviews.apache.org/r/3756/#comment10779>
Can you separate the checks for error code and error message?
trunk/core/src/test/java/org/apache/oozie/TestCoordinatorEngine.java
<https://reviews.apache.org/r/3756/#comment10780>
Can you separate the checks for error code and error message?
trunk/core/src/test/java/org/apache/oozie/executor/jpa/TestCoordJobGetActionsSubsetJPAExecutor.java
<https://reviews.apache.org/r/3756/#comment10782>
Can you please use more descriptive variable names - for example,
filterList?
trunk/core/src/test/java/org/apache/oozie/executor/jpa/TestCoordJobGetActionsSubsetJPAExecutor.java
<https://reviews.apache.org/r/3756/#comment10783>
What is the advantage of having this private method? Can it be reused by
other methods?
If the method is retained, please add comments on what is expected of this
method?
trunk/docs/src/site/twiki/DG_CommandLineTool.twiki
<https://reviews.apache.org/r/3756/#comment10785>
This line is too technical for an application developer. Can it be
simplified to state that when multiple filters are specified, all coordinator
actions that satisfy any one of the filters will be retrieved?
- Santhosh
On 2012-02-03 18:45:55, Virag Kothari wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/3756/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2012-02-03 18:45:55)
bq.
bq.
bq. Review request for oozie.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. https://issues.apache.org/jira/browse/OOZIE-677
bq.
bq.
bq. This addresses bug oozie-677.
bq. https://issues.apache.org/jira/browse/oozie-677
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. trunk/client/src/main/java/org/apache/oozie/cli/OozieCLI.java 1240247
bq. trunk/client/src/main/java/org/apache/oozie/client/OozieClient.java
1240247
bq.
trunk/client/src/main/java/org/apache/oozie/client/rest/RestConstants.java
1240247
bq. trunk/core/src/main/java/org/apache/oozie/BaseEngine.java 1240247
bq. trunk/core/src/main/java/org/apache/oozie/BundleEngine.java 1240247
bq. trunk/core/src/main/java/org/apache/oozie/CoordinatorEngine.java 1240247
bq. trunk/core/src/main/java/org/apache/oozie/DagEngine.java 1240247
bq. trunk/core/src/main/java/org/apache/oozie/ErrorCode.java 1240247
bq.
trunk/core/src/main/java/org/apache/oozie/command/coord/CoordJobXCommand.java
1240247
bq.
trunk/core/src/main/java/org/apache/oozie/executor/jpa/CoordJobGetActionsSubsetJPAExecutor.java
1240247
bq. trunk/core/src/main/java/org/apache/oozie/servlet/V1JobServlet.java
1240247
bq. trunk/core/src/test/java/org/apache/oozie/TestCoordinatorEngine.java
1240247
bq.
trunk/core/src/test/java/org/apache/oozie/executor/jpa/TestCoordJobGetActionsSubsetJPAExecutor.java
1240247
bq.
trunk/core/src/test/java/org/apache/oozie/servlet/MockCoordinatorEngineService.java
1240247
bq. trunk/docs/src/site/twiki/DG_CommandLineTool.twiki 1240247
bq.
bq. Diff: https://reviews.apache.org/r/3756/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq. Test cases added
bq. Checked filter option thru. command line after running cron example
bq.
bq.
bq. Thanks,
bq.
bq. Virag
bq.
bq.
> Add Filter API for status on coordinator actions
> ------------------------------------------------
>
> Key: OOZIE-677
> URL: https://issues.apache.org/jira/browse/OOZIE-677
> Project: Oozie
> Issue Type: Improvement
> Reporter: Virag Kothari
>
> Currently the filter option is only supported for 'oozie jobs'.
> In some cases, they are also required for 'oozie job'. E.g to filter
> coordinator actions for a coordinator job, filter coordinator jobs for a
> bundle etc.
> This jira proposes to provide status filters for coordinator actions on a
> coordinator job
> E.g oozie job -info coordjobid -filter status=RUNNING;status=KILLED
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
