You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@oozie.apache.org by Ryota Egashira <eg...@yahoo-inc.com> on 2014/04/02 03:03:20 UTC

Re: Review Request 19619: OOZIE-1754 add order(sort) option to "oozie job -info"

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

(Updated April 2, 2014, 1:03 a.m.)


Review request for oozie.


Changes
-------

removed -exclude, instead support negative symbol '!=' inside filter, such as "-filter status!=SUCCEEDED"


Bugs: OOZIE-1754
    https://issues.apache.org/jira/browse/OOZIE-1754


Repository: oozie-git


Description
-------

https://issues.apache.org/jira/browse/OOZIE-1754


Diffs (updated)
-----

  client/src/main/java/org/apache/oozie/cli/OozieCLI.java 6dc4a3b 
  client/src/main/java/org/apache/oozie/client/OozieClient.java 6164447 
  core/src/main/java/org/apache/oozie/CoordinatorEngine.java 3f10024 
  core/src/main/java/org/apache/oozie/DagEngine.java 300d6eb 
  core/src/main/java/org/apache/oozie/command/coord/CoordJobXCommand.java 0a030af 
  core/src/main/java/org/apache/oozie/executor/jpa/CoordJobGetActionsSubsetJPAExecutor.java 21506ab 
  core/src/test/java/org/apache/oozie/TestCoordinatorEngine.java 85ef53b 
  core/src/test/java/org/apache/oozie/client/TestOozieCLI.java e90d28a 
  core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java db591e2 
  core/src/test/java/org/apache/oozie/executor/jpa/TestCoordJobGetActionsSubsetJPAExecutor.java da7d16c 
  core/src/test/java/org/apache/oozie/servlet/MockCoordinatorEngineService.java f174b06 
  docs/src/site/twiki/DG_CommandLineTool.twiki ee4c7d9 
  docs/src/site/twiki/WebServicesAPI.twiki 37c0bc0 

Diff: https://reviews.apache.org/r/19619/diff/


Testing
-------

did local test


Thanks,

Ryota Egashira

Re: Review Request 19619: OOZIE-1754 add order(sort) option to "oozie job -info"

Posted by Purshotam Shah <pu...@yahoo-inc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19619/#review39323
-----------------------------------------------------------


Only looked at filter changes.


client/src/main/java/org/apache/oozie/cli/OozieCLI.java
<https://reviews.apache.org/r/19619/#comment71664>

    we should change the example 
    status=<S1>|status!=<S1> and {status=<S2>|status!=<S2> looks incorrect. User should specify =,= or !=,!=. Combination doesn't make sense. 
    



client/src/main/java/org/apache/oozie/cli/OozieCLI.java
<https://reviews.apache.org/r/19619/#comment71665>

    space after , 



core/src/main/java/org/apache/oozie/CoordinatorEngine.java
<https://reviews.apache.org/r/19619/#comment71667>

    We can simply this to.
    
    String filter = POSITIVE_FILTER;
    if(token.contains("!=")) {
    	pair = token.split("!=");
    	filter = NEGATIVE_FILTER;
    }
    else {
    	pair = token.split("=");
    }



core/src/main/java/org/apache/oozie/CoordinatorEngine.java
<https://reviews.apache.org/r/19619/#comment71668>

    We can simply this to
    List<String> filterlist = filterMap.get(filter);
    if (filterlist == null) {
    	filterlist = new ArrayList<String();
    	filterMap.put(filter, filterlist);
    	}
    filterlist.add(statusValue);



core/src/main/java/org/apache/oozie/executor/jpa/CoordJobGetActionsSubsetJPAExecutor.java
<https://reviews.apache.org/r/19619/#comment71669>

    We should only support POSITIVE_FILTER or NEGATIVE_FILTER, not both.
    
    in this case sql will look like  
    
    select * from .... where .. and  a.statusStr IN () and a.statusStr NOT IN ()..
    
    It should be only IN or not IN.



core/src/test/java/org/apache/oozie/TestCoordinatorEngine.java
<https://reviews.apache.org/r/19619/#comment71670>

    This should throw error.



docs/src/site/twiki/DG_CommandLineTool.twiki
<https://reviews.apache.org/r/19619/#comment71675>

    combination of -ve and +ve filter will always return empty list. We should only support -ve or +ve.


- Purshotam Shah


On April 2, 2014, 1:03 a.m., Ryota Egashira wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19619/
> -----------------------------------------------------------
> 
> (Updated April 2, 2014, 1:03 a.m.)
> 
> 
> Review request for oozie.
> 
> 
> Bugs: OOZIE-1754
>     https://issues.apache.org/jira/browse/OOZIE-1754
> 
> 
> Repository: oozie-git
> 
> 
> Description
> -------
> 
> https://issues.apache.org/jira/browse/OOZIE-1754
> 
> 
> Diffs
> -----
> 
>   client/src/main/java/org/apache/oozie/cli/OozieCLI.java 6dc4a3b 
>   client/src/main/java/org/apache/oozie/client/OozieClient.java 6164447 
>   core/src/main/java/org/apache/oozie/CoordinatorEngine.java 3f10024 
>   core/src/main/java/org/apache/oozie/DagEngine.java 300d6eb 
>   core/src/main/java/org/apache/oozie/command/coord/CoordJobXCommand.java 0a030af 
>   core/src/main/java/org/apache/oozie/executor/jpa/CoordJobGetActionsSubsetJPAExecutor.java 21506ab 
>   core/src/test/java/org/apache/oozie/TestCoordinatorEngine.java 85ef53b 
>   core/src/test/java/org/apache/oozie/client/TestOozieCLI.java e90d28a 
>   core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java db591e2 
>   core/src/test/java/org/apache/oozie/executor/jpa/TestCoordJobGetActionsSubsetJPAExecutor.java da7d16c 
>   core/src/test/java/org/apache/oozie/servlet/MockCoordinatorEngineService.java f174b06 
>   docs/src/site/twiki/DG_CommandLineTool.twiki ee4c7d9 
>   docs/src/site/twiki/WebServicesAPI.twiki 37c0bc0 
> 
> Diff: https://reviews.apache.org/r/19619/diff/
> 
> 
> Testing
> -------
> 
> did local test
> 
> 
> Thanks,
> 
> Ryota Egashira
> 
>

Re: Review Request 19619: OOZIE-1754 add order(sort) option to "oozie job -info"

Posted by Rohini Palaniswamy <ro...@gmail.com>.

> On April 7, 2014, 6:04 p.m., Rohini Palaniswamy wrote:
> > core/src/main/java/org/apache/oozie/CoordinatorEngine.java, lines 591-592
> > <https://reviews.apache.org/r/19619/diff/6/?file=548223#file548223line591>
> >
> >     Why is this check there? You can only have positive only or negative only filters right? 
> >     
> >     Can you remove this in the final patch before check in?

 Though it does not make sense and will only be equivalent to status=A, currently no error is thrown if -filter status=A;status!=B is specified. It is very minor problem and ok with it. Can drop this issue. 


- Rohini


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


On April 3, 2014, 9:24 p.m., Ryota Egashira wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19619/
> -----------------------------------------------------------
> 
> (Updated April 3, 2014, 9:24 p.m.)
> 
> 
> Review request for oozie.
> 
> 
> Bugs: OOZIE-1754
>     https://issues.apache.org/jira/browse/OOZIE-1754
> 
> 
> Repository: oozie-git
> 
> 
> Description
> -------
> 
> https://issues.apache.org/jira/browse/OOZIE-1754
> 
> 
> Diffs
> -----
> 
>   client/src/main/java/org/apache/oozie/cli/OozieCLI.java e1f551d 
>   client/src/main/java/org/apache/oozie/client/OozieClient.java 46c4288 
>   core/src/main/java/org/apache/oozie/CoordinatorEngine.java 3f10024 
>   core/src/main/java/org/apache/oozie/DagEngine.java 300d6eb 
>   core/src/main/java/org/apache/oozie/command/coord/CoordJobXCommand.java 0a030af 
>   core/src/main/java/org/apache/oozie/executor/jpa/CoordJobGetActionsSubsetJPAExecutor.java 21506ab 
>   core/src/test/java/org/apache/oozie/TestCoordinatorEngine.java 85ef53b 
>   core/src/test/java/org/apache/oozie/client/TestOozieCLI.java e90d28a 
>   core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java db591e2 
>   core/src/test/java/org/apache/oozie/executor/jpa/TestCoordJobGetActionsSubsetJPAExecutor.java da7d16c 
>   core/src/test/java/org/apache/oozie/servlet/MockCoordinatorEngineService.java f174b06 
>   docs/src/site/twiki/DG_CommandLineTool.twiki 351f0f2 
>   docs/src/site/twiki/WebServicesAPI.twiki 37c0bc0 
> 
> Diff: https://reviews.apache.org/r/19619/diff/
> 
> 
> Testing
> -------
> 
> did local test
> 
> 
> Thanks,
> 
> Ryota Egashira
> 
>

Re: Review Request 19619: OOZIE-1754 add order(sort) option to "oozie job -info"

Posted by Rohini Palaniswamy <ro...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19619/#review39705
-----------------------------------------------------------

Ship it!



core/src/main/java/org/apache/oozie/CoordinatorEngine.java
<https://reviews.apache.org/r/19619/#comment72310>

    Why is this check there? You can only have positive only or negative only filters right? 
    
    Can you remove this in the final patch before check in?


- Rohini Palaniswamy


On April 3, 2014, 9:24 p.m., Ryota Egashira wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/19619/
> -----------------------------------------------------------
> 
> (Updated April 3, 2014, 9:24 p.m.)
> 
> 
> Review request for oozie.
> 
> 
> Bugs: OOZIE-1754
>     https://issues.apache.org/jira/browse/OOZIE-1754
> 
> 
> Repository: oozie-git
> 
> 
> Description
> -------
> 
> https://issues.apache.org/jira/browse/OOZIE-1754
> 
> 
> Diffs
> -----
> 
>   client/src/main/java/org/apache/oozie/cli/OozieCLI.java e1f551d 
>   client/src/main/java/org/apache/oozie/client/OozieClient.java 46c4288 
>   core/src/main/java/org/apache/oozie/CoordinatorEngine.java 3f10024 
>   core/src/main/java/org/apache/oozie/DagEngine.java 300d6eb 
>   core/src/main/java/org/apache/oozie/command/coord/CoordJobXCommand.java 0a030af 
>   core/src/main/java/org/apache/oozie/executor/jpa/CoordJobGetActionsSubsetJPAExecutor.java 21506ab 
>   core/src/test/java/org/apache/oozie/TestCoordinatorEngine.java 85ef53b 
>   core/src/test/java/org/apache/oozie/client/TestOozieCLI.java e90d28a 
>   core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java db591e2 
>   core/src/test/java/org/apache/oozie/executor/jpa/TestCoordJobGetActionsSubsetJPAExecutor.java da7d16c 
>   core/src/test/java/org/apache/oozie/servlet/MockCoordinatorEngineService.java f174b06 
>   docs/src/site/twiki/DG_CommandLineTool.twiki 351f0f2 
>   docs/src/site/twiki/WebServicesAPI.twiki 37c0bc0 
> 
> Diff: https://reviews.apache.org/r/19619/diff/
> 
> 
> Testing
> -------
> 
> did local test
> 
> 
> Thanks,
> 
> Ryota Egashira
> 
>

Re: Review Request 19619: OOZIE-1754 add order(sort) option to "oozie job -info"

Posted by Ryota Egashira <eg...@yahoo-inc.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/19619/
-----------------------------------------------------------

(Updated April 3, 2014, 9:24 p.m.)


Review request for oozie.


Changes
-------

fixed review comments.  
added error check for the case where -filter status=S1;status!=S1, also changed doc


Bugs: OOZIE-1754
    https://issues.apache.org/jira/browse/OOZIE-1754


Repository: oozie-git


Description
-------

https://issues.apache.org/jira/browse/OOZIE-1754


Diffs (updated)
-----

  client/src/main/java/org/apache/oozie/cli/OozieCLI.java e1f551d 
  client/src/main/java/org/apache/oozie/client/OozieClient.java 46c4288 
  core/src/main/java/org/apache/oozie/CoordinatorEngine.java 3f10024 
  core/src/main/java/org/apache/oozie/DagEngine.java 300d6eb 
  core/src/main/java/org/apache/oozie/command/coord/CoordJobXCommand.java 0a030af 
  core/src/main/java/org/apache/oozie/executor/jpa/CoordJobGetActionsSubsetJPAExecutor.java 21506ab 
  core/src/test/java/org/apache/oozie/TestCoordinatorEngine.java 85ef53b 
  core/src/test/java/org/apache/oozie/client/TestOozieCLI.java e90d28a 
  core/src/test/java/org/apache/oozie/command/coord/TestCoordMaterializeTransitionXCommand.java db591e2 
  core/src/test/java/org/apache/oozie/executor/jpa/TestCoordJobGetActionsSubsetJPAExecutor.java da7d16c 
  core/src/test/java/org/apache/oozie/servlet/MockCoordinatorEngineService.java f174b06 
  docs/src/site/twiki/DG_CommandLineTool.twiki 351f0f2 
  docs/src/site/twiki/WebServicesAPI.twiki 37c0bc0 

Diff: https://reviews.apache.org/r/19619/diff/


Testing
-------

did local test


Thanks,

Ryota Egashira