You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@oozie.apache.org by Abhishek Bafna <ba...@gmail.com> on 2017/01/05 05:26:14 UTC

Re: Review Request 54383: LocalOozieClient is missing methods from OozieClient

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

(Updated Jan. 5, 2017, 5:26 a.m.)


Review request for oozie.


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


Repository: oozie-git


Description
-------

LocalOozieClient is missing methods from OozieClient


Diffs (updated)
-----

  client/src/main/java/org/apache/oozie/client/OozieClient.java 12c80cb 
  core/src/main/java/org/apache/oozie/BaseEngine.java 50df897 
  core/src/main/java/org/apache/oozie/BaseLocalOozieClient.java PRE-CREATION 
  core/src/main/java/org/apache/oozie/LocalOozieClient.java f734f76 
  core/src/main/java/org/apache/oozie/LocalOozieClientBundle.java PRE-CREATION 
  core/src/main/java/org/apache/oozie/LocalOozieClientCoord.java 32b0cd0 
  core/src/main/java/org/apache/oozie/OozieJsonFactory.java PRE-CREATION 
  core/src/main/java/org/apache/oozie/servlet/V0JobsServlet.java 2c79ef0 
  core/src/main/java/org/apache/oozie/servlet/V1JobsServlet.java 80c8ec4 
  core/src/test/java/org/apache/oozie/TestLocalOozieClientCoord.java 4decd52 

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


Testing
-------


Thanks,

Abhishek Bafna


Re: Review Request 54383: LocalOozieClient is missing methods from OozieClient

Posted by András Piros <an...@cloudera.com>.

> On March 24, 2017, 12:24 p.m., Peter Bacsko wrote:
> > core/src/main/java/org/apache/oozie/BaseLocalOozieClient.java
> > Lines 345 (patched)
> > <https://reviews.apache.org/r/54383/diff/6/?file=1597608#file1597608line345>
> >
> >     Please refactor these nested classes (make them non-nested). The whole class is too big with these classes being defined here.

+1 on extracting nested classes, and applying SRP.


> On March 24, 2017, 12:24 p.m., Peter Bacsko wrote:
> > core/src/main/java/org/apache/oozie/BaseLocalOozieClient.java
> > Lines 453 (patched)
> > <https://reviews.apache.org/r/54383/diff/6/?file=1597608#file1597608line453>
> >
> >     As I can see it, this is not the visitor pattern.
> >     
> >     Traditionally, when you use the visitor pattern, the classes that are to be visited implement the accept() method which takes the visitor implementation as an argument. Then, if the class contains other classes that can be visited, it calls the accept() method on the embedded object again, effectively passing the visitor implementation down the call stack.
> >     
> >     Also, the visitation begins by calling the accept() method on the top-most (external) object.
> >     
> >     There is a very good, simple example here:
> >     https://en.wikipedia.org/wiki/Visitor_pattern#Java_example
> >     
> >     I think this is just having different implementations, which are hidden under an interface (which is good)/
> >     
> >     I suggest doing the following rename:
> >     - OozieActionVisitor --> OozieActionHandler
> >     - AbstractOozieActionVisitor --> AbstractOozieActionHandler
> >     - Killing/Resuming/SuspendingVisitor --> Killing/Resuming/SuspendingHandler

Visitor pattern can be heavyweight here, agreed. Renaming `*Visitor` to `*Handler` would remove confusion.


- Andr�s


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


On Jan. 5, 2017, 5:34 p.m., Abhishek Bafna wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54383/
> -----------------------------------------------------------
> 
> (Updated Jan. 5, 2017, 5:34 p.m.)
> 
> 
> Review request for oozie.
> 
> 
> Bugs: OOZIE-2751
>     https://issues.apache.org/jira/browse/OOZIE-2751
> 
> 
> Repository: oozie-git
> 
> 
> Description
> -------
> 
> LocalOozieClient is missing methods from OozieClient
> 
> 
> Diffs
> -----
> 
>   client/src/main/java/org/apache/oozie/client/OozieClient.java 12c80cb 
>   core/src/main/java/org/apache/oozie/BaseEngine.java 50df897 
>   core/src/main/java/org/apache/oozie/BaseLocalOozieClient.java PRE-CREATION 
>   core/src/main/java/org/apache/oozie/LocalOozieClient.java f734f76 
>   core/src/main/java/org/apache/oozie/LocalOozieClientBundle.java PRE-CREATION 
>   core/src/main/java/org/apache/oozie/LocalOozieClientCoord.java 32b0cd0 
>   core/src/main/java/org/apache/oozie/OozieJsonFactory.java PRE-CREATION 
>   core/src/main/java/org/apache/oozie/servlet/V0JobsServlet.java 2c79ef0 
>   core/src/main/java/org/apache/oozie/servlet/V1JobsServlet.java 80c8ec4 
>   core/src/test/java/org/apache/oozie/TestLocalOozieClientCoord.java 4decd52 
> 
> 
> Diff: https://reviews.apache.org/r/54383/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Abhishek Bafna
> 
>


Re: Review Request 54383: LocalOozieClient is missing methods from OozieClient

Posted by Abhishek Bafna <ba...@gmail.com>.

> On March 24, 2017, 12:24 p.m., Peter Bacsko wrote:
> > core/src/main/java/org/apache/oozie/BaseLocalOozieClient.java
> > Lines 457 (patched)
> > <https://reviews.apache.org/r/54383/diff/6/?file=1597608#file1597608line457>
> >
> >     Extract cases to constants.
> >     
> >     Optional: if it's not too much work, pls also take care of replacing the other occurrences.

The patch is already big, so it would be better to not touch other functionality.


> On March 24, 2017, 12:24 p.m., Peter Bacsko wrote:
> > core/src/main/java/org/apache/oozie/LocalOozieClientBundle.java
> > Lines 79 (patched)
> > <https://reviews.apache.org/r/54383/diff/6/?file=1597610#file1597610line79>
> >
> >     Non-typesafe cast. Do we even need to cast here at all?

This is because of super-type vs sub-type. Same as other places.


- Abhishek


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


On Jan. 5, 2017, 5:34 p.m., Abhishek Bafna wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54383/
> -----------------------------------------------------------
> 
> (Updated Jan. 5, 2017, 5:34 p.m.)
> 
> 
> Review request for oozie.
> 
> 
> Bugs: OOZIE-2751
>     https://issues.apache.org/jira/browse/OOZIE-2751
> 
> 
> Repository: oozie-git
> 
> 
> Description
> -------
> 
> LocalOozieClient is missing methods from OozieClient
> 
> 
> Diffs
> -----
> 
>   client/src/main/java/org/apache/oozie/client/OozieClient.java 12c80cb 
>   core/src/main/java/org/apache/oozie/BaseEngine.java 50df897 
>   core/src/main/java/org/apache/oozie/BaseLocalOozieClient.java PRE-CREATION 
>   core/src/main/java/org/apache/oozie/LocalOozieClient.java f734f76 
>   core/src/main/java/org/apache/oozie/LocalOozieClientBundle.java PRE-CREATION 
>   core/src/main/java/org/apache/oozie/LocalOozieClientCoord.java 32b0cd0 
>   core/src/main/java/org/apache/oozie/OozieJsonFactory.java PRE-CREATION 
>   core/src/main/java/org/apache/oozie/servlet/V0JobsServlet.java 2c79ef0 
>   core/src/main/java/org/apache/oozie/servlet/V1JobsServlet.java 80c8ec4 
>   core/src/test/java/org/apache/oozie/TestLocalOozieClientCoord.java 4decd52 
> 
> 
> Diff: https://reviews.apache.org/r/54383/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Abhishek Bafna
> 
>


Re: Review Request 54383: LocalOozieClient is missing methods from OozieClient

Posted by Peter Bacsko <pb...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54383/#review169999
-----------------------------------------------------------




core/src/main/java/org/apache/oozie/BaseLocalOozieClient.java
Lines 54 (patched)
<https://reviews.apache.org/r/54383/#comment242725>

    Minor: Preconditions.checkNotNull() is useful here IMO.



core/src/main/java/org/apache/oozie/BaseLocalOozieClient.java
Lines 68 (patched)
<https://reviews.apache.org/r/54383/#comment242722>

    Is this method ever called? If not, it should be abstract.
    
    If it is, please add a small "// no-op" comment to indicate that it is a valid implementation.



core/src/main/java/org/apache/oozie/BaseLocalOozieClient.java
Lines 82 (patched)
<https://reviews.apache.org/r/54383/#comment242723>

    Is this method ever called? If not, it should be abstract.
    
    If it is, please add a small "// no-op" comment to indicate that it is a valid implementation.



core/src/main/java/org/apache/oozie/BaseLocalOozieClient.java
Lines 91 (patched)
<https://reviews.apache.org/r/54383/#comment242724>

    Is this method ever called? If not, it should be abstract.
    
    If it is, please add a small "// no-op" comment to indicate that it is a valid implementation.



core/src/main/java/org/apache/oozie/BaseLocalOozieClient.java
Lines 95 (patched)
<https://reviews.apache.org/r/54383/#comment242727>

    Unnecessary @SuppressWarnings



core/src/main/java/org/apache/oozie/BaseLocalOozieClient.java
Lines 345 (patched)
<https://reviews.apache.org/r/54383/#comment242726>

    Please refactor these nested classes (make them non-nested). The whole class is too big with these classes being defined here.



core/src/main/java/org/apache/oozie/BaseLocalOozieClient.java
Lines 453 (patched)
<https://reviews.apache.org/r/54383/#comment242739>

    As I can see it, this is not the visitor pattern.
    
    Traditionally, when you use the visitor pattern, the classes that are to be visited implement the accept() method which takes the visitor implementation as an argument. Then, if the class contains other classes that can be visited, it calls the accept() method on the embedded object again, effectively passing the visitor implementation down the call stack.
    
    Also, the visitation begins by calling the accept() method on the top-most (external) object.
    
    There is a very good, simple example here:
    https://en.wikipedia.org/wiki/Visitor_pattern#Java_example
    
    I think this is just having different implementations, which are hidden under an interface (which is good)/
    
    I suggest doing the following rename:
    - OozieActionVisitor --> OozieActionHandler
    - AbstractOozieActionVisitor --> AbstractOozieActionHandler
    - Killing/Resuming/SuspendingVisitor --> Killing/Resuming/SuspendingHandler



core/src/main/java/org/apache/oozie/BaseLocalOozieClient.java
Lines 457 (patched)
<https://reviews.apache.org/r/54383/#comment242728>

    Extract cases to constants.
    
    Optional: if it's not too much work, pls also take care of replacing the other occurrences.



core/src/main/java/org/apache/oozie/LocalOozieClientBundle.java
Lines 79 (patched)
<https://reviews.apache.org/r/54383/#comment242731>

    Non-typesafe cast. Do we even need to cast here at all?



core/src/main/java/org/apache/oozie/LocalOozieClientBundle.java
Lines 88 (patched)
<https://reviews.apache.org/r/54383/#comment242730>

    Non-typesafe cast. Do we even need to cast here at all?



core/src/main/java/org/apache/oozie/LocalOozieClientCoord.java
Lines 263 (patched)
<https://reviews.apache.org/r/54383/#comment242732>

    Non-typesafe cast. Do we even need to cast here at all?



core/src/main/java/org/apache/oozie/LocalOozieClientCoord.java
Lines 272 (patched)
<https://reviews.apache.org/r/54383/#comment242733>

    Non-typesafe cast. Do we even need to cast here at all?



core/src/main/java/org/apache/oozie/LocalOozieClientCoord.java
Lines 281 (patched)
<https://reviews.apache.org/r/54383/#comment242734>

    Non-typesafe cast. Do we even need to cast here at all?



core/src/main/java/org/apache/oozie/OozieJsonFactory.java
Lines 24 (patched)
<https://reviews.apache.org/r/54383/#comment242735>

    If it's meant to be an utility class, then it should be final instead of abstract.
    
    Also, consider adding a private constuctor to prevent instantiation.



core/src/main/java/org/apache/oozie/servlet/V1JobsServlet.java
Line 316 (original), 314 (patched)
<https://reviews.apache.org/r/54383/#comment242736>

    Minor: trailing whitespace



core/src/main/java/org/apache/oozie/servlet/V1JobsServlet.java
Line 349 (original), 342 (patched)
<https://reviews.apache.org/r/54383/#comment242737>

    Minor: trailing whitespace



core/src/main/java/org/apache/oozie/servlet/V1JobsServlet.java
Line 378 (original), 366 (patched)
<https://reviews.apache.org/r/54383/#comment242738>

    Minor: trailing whitespace


- Peter Bacsko


On jan. 5, 2017, 5:34 du, Abhishek Bafna wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54383/
> -----------------------------------------------------------
> 
> (Updated jan. 5, 2017, 5:34 du)
> 
> 
> Review request for oozie.
> 
> 
> Bugs: OOZIE-2751
>     https://issues.apache.org/jira/browse/OOZIE-2751
> 
> 
> Repository: oozie-git
> 
> 
> Description
> -------
> 
> LocalOozieClient is missing methods from OozieClient
> 
> 
> Diffs
> -----
> 
>   client/src/main/java/org/apache/oozie/client/OozieClient.java 12c80cb 
>   core/src/main/java/org/apache/oozie/BaseEngine.java 50df897 
>   core/src/main/java/org/apache/oozie/BaseLocalOozieClient.java PRE-CREATION 
>   core/src/main/java/org/apache/oozie/LocalOozieClient.java f734f76 
>   core/src/main/java/org/apache/oozie/LocalOozieClientBundle.java PRE-CREATION 
>   core/src/main/java/org/apache/oozie/LocalOozieClientCoord.java 32b0cd0 
>   core/src/main/java/org/apache/oozie/OozieJsonFactory.java PRE-CREATION 
>   core/src/main/java/org/apache/oozie/servlet/V0JobsServlet.java 2c79ef0 
>   core/src/main/java/org/apache/oozie/servlet/V1JobsServlet.java 80c8ec4 
>   core/src/test/java/org/apache/oozie/TestLocalOozieClientCoord.java 4decd52 
> 
> 
> Diff: https://reviews.apache.org/r/54383/diff/6/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Abhishek Bafna
> 
>


Re: Review Request 54383: LocalOozieClient is missing methods from OozieClient

Posted by András Piros <an...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54383/#review160609
-----------------------------------------------------------


Ship it!




Ship It!

- Andr�s Piros


On Jan. 5, 2017, 5:34 p.m., Abhishek Bafna wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54383/
> -----------------------------------------------------------
> 
> (Updated Jan. 5, 2017, 5:34 p.m.)
> 
> 
> Review request for oozie.
> 
> 
> Bugs: OOZIE-2751
>     https://issues.apache.org/jira/browse/OOZIE-2751
> 
> 
> Repository: oozie-git
> 
> 
> Description
> -------
> 
> LocalOozieClient is missing methods from OozieClient
> 
> 
> Diffs
> -----
> 
>   client/src/main/java/org/apache/oozie/client/OozieClient.java 12c80cb 
>   core/src/main/java/org/apache/oozie/BaseEngine.java 50df897 
>   core/src/main/java/org/apache/oozie/BaseLocalOozieClient.java PRE-CREATION 
>   core/src/main/java/org/apache/oozie/LocalOozieClient.java f734f76 
>   core/src/main/java/org/apache/oozie/LocalOozieClientBundle.java PRE-CREATION 
>   core/src/main/java/org/apache/oozie/LocalOozieClientCoord.java 32b0cd0 
>   core/src/main/java/org/apache/oozie/OozieJsonFactory.java PRE-CREATION 
>   core/src/main/java/org/apache/oozie/servlet/V0JobsServlet.java 2c79ef0 
>   core/src/main/java/org/apache/oozie/servlet/V1JobsServlet.java 80c8ec4 
>   core/src/test/java/org/apache/oozie/TestLocalOozieClientCoord.java 4decd52 
> 
> Diff: https://reviews.apache.org/r/54383/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Abhishek Bafna
> 
>


Re: Review Request 54383: LocalOozieClient is missing methods from OozieClient

Posted by Robert Kanter <rk...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54383/#review174226
-----------------------------------------------------------


Ship it!




Ship It!

- Robert Kanter


On May 8, 2017, 4:41 p.m., Abhishek Bafna wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54383/
> -----------------------------------------------------------
> 
> (Updated May 8, 2017, 4:41 p.m.)
> 
> 
> Review request for oozie.
> 
> 
> Bugs: OOZIE-2751
>     https://issues.apache.org/jira/browse/OOZIE-2751
> 
> 
> Repository: oozie-git
> 
> 
> Description
> -------
> 
> LocalOozieClient is missing methods from OozieClient
> 
> 
> Diffs
> -----
> 
>   client/src/main/java/org/apache/oozie/client/OozieClient.java 7370808 
>   client/src/main/java/org/apache/oozie/client/rest/RestConstants.java f477531 
>   core/src/main/java/org/apache/oozie/BaseEngine.java 2780ec2 
>   core/src/main/java/org/apache/oozie/BaseLocalOozieClient.java PRE-CREATION 
>   core/src/main/java/org/apache/oozie/LocalOozieClient.java f734f76 
>   core/src/main/java/org/apache/oozie/LocalOozieClientBundle.java PRE-CREATION 
>   core/src/main/java/org/apache/oozie/LocalOozieClientCoord.java 32b0cd0 
>   core/src/main/java/org/apache/oozie/OozieClientOperationHandler.java PRE-CREATION 
>   core/src/main/java/org/apache/oozie/OozieJsonFactory.java PRE-CREATION 
>   core/src/main/java/org/apache/oozie/local/LocalOozie.java bf1b0db 
>   core/src/main/java/org/apache/oozie/servlet/V0JobsServlet.java 2c79ef0 
>   core/src/main/java/org/apache/oozie/servlet/V1JobsServlet.java 80c8ec4 
>   core/src/test/java/org/apache/oozie/TestLocalOozieClientCoord.java 4decd52 
> 
> 
> Diff: https://reviews.apache.org/r/54383/diff/8/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Abhishek Bafna
> 
>


Re: Review Request 54383: LocalOozieClient is missing methods from OozieClient

Posted by Abhishek Bafna <ba...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54383/
-----------------------------------------------------------

(Updated May 8, 2017, 4:41 p.m.)


Review request for oozie.


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


Repository: oozie-git


Description
-------

LocalOozieClient is missing methods from OozieClient


Diffs (updated)
-----

  client/src/main/java/org/apache/oozie/client/OozieClient.java 7370808 
  client/src/main/java/org/apache/oozie/client/rest/RestConstants.java f477531 
  core/src/main/java/org/apache/oozie/BaseEngine.java 2780ec2 
  core/src/main/java/org/apache/oozie/BaseLocalOozieClient.java PRE-CREATION 
  core/src/main/java/org/apache/oozie/LocalOozieClient.java f734f76 
  core/src/main/java/org/apache/oozie/LocalOozieClientBundle.java PRE-CREATION 
  core/src/main/java/org/apache/oozie/LocalOozieClientCoord.java 32b0cd0 
  core/src/main/java/org/apache/oozie/OozieClientOperationHandler.java PRE-CREATION 
  core/src/main/java/org/apache/oozie/OozieJsonFactory.java PRE-CREATION 
  core/src/main/java/org/apache/oozie/local/LocalOozie.java bf1b0db 
  core/src/main/java/org/apache/oozie/servlet/V0JobsServlet.java 2c79ef0 
  core/src/main/java/org/apache/oozie/servlet/V1JobsServlet.java 80c8ec4 
  core/src/test/java/org/apache/oozie/TestLocalOozieClientCoord.java 4decd52 


Diff: https://reviews.apache.org/r/54383/diff/8/

Changes: https://reviews.apache.org/r/54383/diff/7-8/


Testing
-------


Thanks,

Abhishek Bafna


Re: Review Request 54383: LocalOozieClient is missing methods from OozieClient

Posted by Robert Kanter <rk...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54383/#review174067
-----------------------------------------------------------




core/src/main/java/org/apache/oozie/BaseLocalOozieClient.java
Lines 92 (patched)
<https://reviews.apache.org/r/54383/#comment247169>

    This will cause an NPE if someone tries to use this.  It would be nicer to return something like an empty string.  (That's also more consistent with getHeaderNames(), which returns an empty iterator).



core/src/main/java/org/apache/oozie/BaseLocalOozieClient.java
Lines 461-464 (patched)
<https://reviews.apache.org/r/54383/#comment247170>

    Can you move this higher up near the other related methods (e.g. getHeader(), getHeaderNames(), etc).  Also, why does this one throw an UnsupportedOperationException when the others don't?



core/src/main/java/org/apache/oozie/LocalOozieClientBundle.java
Lines 47 (patched)
<https://reviews.apache.org/r/54383/#comment247171>

    Can you add a method to LocalOozie for creating a LocalOozieClientBundle?
    
    If you look at https://github.com/apache/oozie/blob/master/core/src/main/java/org/apache/oozie/local/LocalOozie.java, you can see there are some methods for creating the WF and Coord local clients, but we need to add some for this new bundle local client.
    
    Also please update any Javadocs in LocalOozie that might be out of date.  There's a lot of lists of NOP methods.


- Robert Kanter


On April 24, 2017, 1:22 p.m., Abhishek Bafna wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54383/
> -----------------------------------------------------------
> 
> (Updated April 24, 2017, 1:22 p.m.)
> 
> 
> Review request for oozie.
> 
> 
> Bugs: OOZIE-2751
>     https://issues.apache.org/jira/browse/OOZIE-2751
> 
> 
> Repository: oozie-git
> 
> 
> Description
> -------
> 
> LocalOozieClient is missing methods from OozieClient
> 
> 
> Diffs
> -----
> 
>   client/src/main/java/org/apache/oozie/client/OozieClient.java 7370808 
>   client/src/main/java/org/apache/oozie/client/rest/RestConstants.java f477531 
>   core/src/main/java/org/apache/oozie/BaseEngine.java 2780ec2 
>   core/src/main/java/org/apache/oozie/BaseLocalOozieClient.java PRE-CREATION 
>   core/src/main/java/org/apache/oozie/LocalOozieClient.java f734f76 
>   core/src/main/java/org/apache/oozie/LocalOozieClientBundle.java PRE-CREATION 
>   core/src/main/java/org/apache/oozie/LocalOozieClientCoord.java 32b0cd0 
>   core/src/main/java/org/apache/oozie/OozieClientOperationHandler.java PRE-CREATION 
>   core/src/main/java/org/apache/oozie/OozieJsonFactory.java PRE-CREATION 
>   core/src/main/java/org/apache/oozie/servlet/V0JobsServlet.java 2c79ef0 
>   core/src/main/java/org/apache/oozie/servlet/V1JobsServlet.java 80c8ec4 
>   core/src/test/java/org/apache/oozie/TestLocalOozieClientCoord.java 4decd52 
> 
> 
> Diff: https://reviews.apache.org/r/54383/diff/7/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Abhishek Bafna
> 
>


Re: Review Request 54383: LocalOozieClient is missing methods from OozieClient

Posted by Abhishek Bafna <ba...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54383/
-----------------------------------------------------------

(Updated April 24, 2017, 1:22 p.m.)


Review request for oozie.


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


Repository: oozie-git


Description
-------

LocalOozieClient is missing methods from OozieClient


Diffs (updated)
-----

  client/src/main/java/org/apache/oozie/client/OozieClient.java 7370808 
  client/src/main/java/org/apache/oozie/client/rest/RestConstants.java f477531 
  core/src/main/java/org/apache/oozie/BaseEngine.java 2780ec2 
  core/src/main/java/org/apache/oozie/BaseLocalOozieClient.java PRE-CREATION 
  core/src/main/java/org/apache/oozie/LocalOozieClient.java f734f76 
  core/src/main/java/org/apache/oozie/LocalOozieClientBundle.java PRE-CREATION 
  core/src/main/java/org/apache/oozie/LocalOozieClientCoord.java 32b0cd0 
  core/src/main/java/org/apache/oozie/OozieClientOperationHandler.java PRE-CREATION 
  core/src/main/java/org/apache/oozie/OozieJsonFactory.java PRE-CREATION 
  core/src/main/java/org/apache/oozie/servlet/V0JobsServlet.java 2c79ef0 
  core/src/main/java/org/apache/oozie/servlet/V1JobsServlet.java 80c8ec4 
  core/src/test/java/org/apache/oozie/TestLocalOozieClientCoord.java 4decd52 


Diff: https://reviews.apache.org/r/54383/diff/7/

Changes: https://reviews.apache.org/r/54383/diff/6-7/


Testing
-------


Thanks,

Abhishek Bafna


Re: Review Request 54383: LocalOozieClient is missing methods from OozieClient

Posted by Abhishek Bafna <ba...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54383/
-----------------------------------------------------------

(Updated Jan. 5, 2017, 5:34 p.m.)


Review request for oozie.


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


Repository: oozie-git


Description
-------

LocalOozieClient is missing methods from OozieClient


Diffs (updated)
-----

  client/src/main/java/org/apache/oozie/client/OozieClient.java 12c80cb 
  core/src/main/java/org/apache/oozie/BaseEngine.java 50df897 
  core/src/main/java/org/apache/oozie/BaseLocalOozieClient.java PRE-CREATION 
  core/src/main/java/org/apache/oozie/LocalOozieClient.java f734f76 
  core/src/main/java/org/apache/oozie/LocalOozieClientBundle.java PRE-CREATION 
  core/src/main/java/org/apache/oozie/LocalOozieClientCoord.java 32b0cd0 
  core/src/main/java/org/apache/oozie/OozieJsonFactory.java PRE-CREATION 
  core/src/main/java/org/apache/oozie/servlet/V0JobsServlet.java 2c79ef0 
  core/src/main/java/org/apache/oozie/servlet/V1JobsServlet.java 80c8ec4 
  core/src/test/java/org/apache/oozie/TestLocalOozieClientCoord.java 4decd52 

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


Testing
-------


Thanks,

Abhishek Bafna


Re: Review Request 54383: LocalOozieClient is missing methods from OozieClient

Posted by András Piros <an...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/54383/#review160582
-----------------------------------------------------------




core/src/main/java/org/apache/oozie/BaseEngine.java (lines 338 - 341)
<https://reviews.apache.org/r/54383/#comment231715>

    Why not just call `throwNoOp()` and do not consider its return value? I think we don't need this method, as it is mainly a duplicate of `throwNoOp()`.


- Andr�s Piros


On Jan. 5, 2017, 5:26 a.m., Abhishek Bafna wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/54383/
> -----------------------------------------------------------
> 
> (Updated Jan. 5, 2017, 5:26 a.m.)
> 
> 
> Review request for oozie.
> 
> 
> Bugs: OOZIE-2751
>     https://issues.apache.org/jira/browse/OOZIE-2751
> 
> 
> Repository: oozie-git
> 
> 
> Description
> -------
> 
> LocalOozieClient is missing methods from OozieClient
> 
> 
> Diffs
> -----
> 
>   client/src/main/java/org/apache/oozie/client/OozieClient.java 12c80cb 
>   core/src/main/java/org/apache/oozie/BaseEngine.java 50df897 
>   core/src/main/java/org/apache/oozie/BaseLocalOozieClient.java PRE-CREATION 
>   core/src/main/java/org/apache/oozie/LocalOozieClient.java f734f76 
>   core/src/main/java/org/apache/oozie/LocalOozieClientBundle.java PRE-CREATION 
>   core/src/main/java/org/apache/oozie/LocalOozieClientCoord.java 32b0cd0 
>   core/src/main/java/org/apache/oozie/OozieJsonFactory.java PRE-CREATION 
>   core/src/main/java/org/apache/oozie/servlet/V0JobsServlet.java 2c79ef0 
>   core/src/main/java/org/apache/oozie/servlet/V1JobsServlet.java 80c8ec4 
>   core/src/test/java/org/apache/oozie/TestLocalOozieClientCoord.java 4decd52 
> 
> Diff: https://reviews.apache.org/r/54383/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Abhishek Bafna
> 
>