----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54383/#review158328 -----------------------------------------------------------
core/src/main/java/org/apache/oozie/BaseLocalOozieClient.java (lines 234 - 258) <https://reviews.apache.org/r/54383/#comment229103> I think `BaseEngine` should contain the three abstract methods `killJobs()`, `submitJobs()`, and `resumeJobs()` - we wouldn't have to cast 9x. What moreover bothers me here that we have 3x almost exactly the same code - differences are only in the method calls to `? extends BaseEngine`. My idea is here to have an inner class w/ all the four incoming parameters as `final` fields initialized in constructor and having three methods like `kill()`, `submit()`, and `resume()` - all the three would call the one single `private JSONObject perform(Callable engineCallable) throws OozieClientException` that has the `switch case` statements and accepts a specific `Callable` that calls `? extends BaseEngine`. core/src/main/java/org/apache/oozie/BaseLocalOozieClient.java (line 323) <https://reviews.apache.org/r/54383/#comment229102> I would extract following to a method: ``` private void throwNoOpOozieClientException() throws OozieClientException { throw new OozieClientException(ErrorCode.E0301.toString(), "no-op"); } ``` and wouldn't make any premature optimizations - modern JVMs like HotSpot will probably inline that method call anyway. - András Piros On Dec. 7, 2016, 4:19 a.m., Abhishek Bafna wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/54383/ > ----------------------------------------------------------- > > (Updated Dec. 7, 2016, 4:19 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/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 > > Diff: https://reviews.apache.org/r/54383/diff/ > > > Testing > ------- > > > Thanks, > > Abhishek Bafna > >
