----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/54383/#review158020 -----------------------------------------------------------
Some small refactoring advices - but in general, nice job! core/src/main/java/org/apache/oozie/BaseLocalOozieClient.java (line 38) <https://reviews.apache.org/r/54383/#comment228675> Why not make `final`? core/src/main/java/org/apache/oozie/BaseLocalOozieClient.java (line 84) <https://reviews.apache.org/r/54383/#comment228674> Collections.emptySet().iterator() core/src/main/java/org/apache/oozie/BaseLocalOozieClient.java (line 229) <https://reviews.apache.org/r/54383/#comment228684> Nice to see default path also :) core/src/main/java/org/apache/oozie/BaseLocalOozieClient.java (lines 234 - 258) <https://reviews.apache.org/r/54383/#comment228686> I would extract this to another - generic - method. The difference is only within casting to that generic type, and applying the logic method - that can be a `Function<F, T>` applying `engine.killJobs()` and `getXXXXJsonObject()`: https://google.github.io/guava/releases/19.0/api/docs/com/google/common/base/Function.html core/src/main/java/org/apache/oozie/BaseLocalOozieClient.java (lines 260 - 284) <https://reviews.apache.org/r/54383/#comment228687> I would extract this to another - generic - method. The difference is only within casting to that generic type, and applying the logic method - that can be a `Function<F, T>` applying `engine.suspendJobs()` and `getXXXXJsonObject()`: https://google.github.io/guava/releases/19.0/api/docs/com/google/common/base/Function.html core/src/main/java/org/apache/oozie/BaseLocalOozieClient.java (lines 286 - 310) <https://reviews.apache.org/r/54383/#comment228688> I would extract this to another - generic - method. The difference is only within casting to that generic type, and applying the logic method - that can be a `Function<F, T>` applying `engine.resumeJobs()` and `getXXXXJsonObject()`: https://google.github.io/guava/releases/19.0/api/docs/com/google/common/base/Function.html core/src/main/java/org/apache/oozie/BaseLocalOozieClient.java (line 323) <https://reviews.apache.org/r/54383/#comment228676> Extract method core/src/main/java/org/apache/oozie/BaseLocalOozieClient.java (line 328) <https://reviews.apache.org/r/54383/#comment228677> Extract method core/src/main/java/org/apache/oozie/BaseLocalOozieClient.java (line 333) <https://reviews.apache.org/r/54383/#comment228680> Extract method core/src/main/java/org/apache/oozie/BaseLocalOozieClient.java (line 338) <https://reviews.apache.org/r/54383/#comment228681> Extract method core/src/main/java/org/apache/oozie/BaseLocalOozieClient.java (line 343) <https://reviews.apache.org/r/54383/#comment228682> Extract method core/src/main/java/org/apache/oozie/BaseLocalOozieClient.java (line 348) <https://reviews.apache.org/r/54383/#comment228683> Extract method core/src/main/java/org/apache/oozie/BaseLocalOozieClient.java (lines 351 - 376) <https://reviews.apache.org/r/54383/#comment228678> Would be better to extract to an Abstract Factory. core/src/main/java/org/apache/oozie/LocalOozieClientBundle.java (line 49) <https://reviews.apache.org/r/54383/#comment228689> I like it :) - András Piros On Dec. 5, 2016, 5:05 p.m., Abhishek Bafna wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/54383/ > ----------------------------------------------------------- > > (Updated Dec. 5, 2016, 5:05 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/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 > > Diff: https://reviews.apache.org/r/54383/diff/ > > > Testing > ------- > > > Thanks, > > Abhishek Bafna > >
