----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/68237/#review207853 -----------------------------------------------------------
core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java Lines 1001-1003 (original), 1001-1003 (patched) <https://reviews.apache.org/r/68237/#comment291404> Very similar String formatting can be found three times in the code. Would it be possible to extract it to a separate method? core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java Lines 1103-1107 (original), 1102-1107 (patched) <https://reviews.apache.org/r/68237/#comment291405> Second oocurance of the similar String formatting. core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java Lines 1289-1290 (original), 1289-1290 (patched) <https://reviews.apache.org/r/68237/#comment291406> Third occurance of the similar String formatting. core/src/main/java/org/apache/oozie/action/hadoop/MapReduceActionExecutor.java Line 24 (original), 24 (patched) <https://reviews.apache.org/r/68237/#comment291400> Please remove * import core/src/main/java/org/apache/oozie/action/hadoop/MapReduceActionExecutor.java Lines 26 (patched) <https://reviews.apache.org/r/68237/#comment291401> Please remove * import core/src/main/java/org/apache/oozie/action/hadoop/MapReduceActionExecutor.java Line 37 (original), 38 (patched) <https://reviews.apache.org/r/68237/#comment291402> Please remove * import core/src/main/java/org/apache/oozie/action/hadoop/MapReduceActionExecutor.java Lines 42 (patched) <https://reviews.apache.org/r/68237/#comment291403> Please remove * import core/src/test/java/org/apache/oozie/action/hadoop/TestYarnApplicationIdComparator.java Lines 45 (patched) <https://reviews.apache.org/r/68237/#comment291407> Should be application_a_b and application_c_d if we want to test the parsing of the middle part. Maybe a separate test could test a misspelled prefix like applicationId_11111_222 applicationId_33333_444. core/src/test/java/org/apache/oozie/action/hadoop/TestYarnApplicationIdComparator.java Lines 47 (patched) <https://reviews.apache.org/r/68237/#comment291412> Which part of the code should throw an ArrayIndexOutOfBoundsException? Why not check IndexOutOfBoundsException instead? sharelib/streaming/src/test/java/org/apache/oozie/action/hadoop/TestMapReduceActionExecutor.java Lines 1389 (patched) <https://reviews.apache.org/r/68237/#comment291414> extra space indentation sharelib/streaming/src/test/java/org/apache/oozie/action/hadoop/TestMapReduceActionExecutor.java Lines 1436 (patched) <https://reviews.apache.org/r/68237/#comment291413> isEmpty() could be used instead of size() == 0 - Andras Salamon On Aug. 23, 2018, 6:36 p.m., András Piros wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/68237/ > ----------------------------------------------------------- > > (Updated Aug. 23, 2018, 6:36 p.m.) > > > Review request for oozie, Andras Salamon, Attila Sasvari, Kinga Marton, and > Peter Bacsko. > > > Repository: oozie-git > > > Description > ------- > > OOZIE-3298 OYA: external ID is not filled properly and failing MR job is > treated as SUCCEEDED > > > Diffs > ----- > > core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java > 8f0f244013932476d8ae454d224f235948529b34 > > core/src/main/java/org/apache/oozie/action/hadoop/MapReduceActionExecutor.java > 83a23f5220aa72ba15edc8b98ef80a74213fcee8 > > core/src/test/java/org/apache/oozie/action/hadoop/ActionExecutorTestCase.java > f39bba2c691435354dac6da7794e5142b511d937 > > core/src/test/java/org/apache/oozie/action/hadoop/TestJavaActionExecutor.java > a31079a41d30677d35a253a4a69505c21aa585f6 > > core/src/test/java/org/apache/oozie/action/hadoop/TestYarnApplicationIdComparator.java > PRE-CREATION > > core/src/test/java/org/apache/oozie/action/hadoop/TestYarnApplicationIdFinder.java > PRE-CREATION > > core/src/test/java/org/apache/oozie/action/oozie/TestSubWorkflowActionExecutor.java > 893405e38ad71d22460768b76ed03ac4d9a0b95d > > sharelib/streaming/src/test/java/org/apache/oozie/action/hadoop/TestMapReduceActionExecutor.java > f460b6bd11f60dfb397c6bba82be1427c2d1b570 > > > Diff: https://reviews.apache.org/r/68237/diff/4/ > > > Testing > ------- > > Tested on a real cluster, plus added test cases to > `TestMapReduceActionExecutor` and new test classes. > > Please note that I could find no proper way of actually getting MapReduce to > start a new job while using `OozieClient#submit()` - apparently no YARN child > application is created. Please advise what's the best way to advance, maybe > call `JobClient#submitJob()` directly from > `ActionExecutorTestCase#startWorkflowAndFailChildMRJob()`. > > > Thanks, > > András Piros > >
