----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/66243/#review199874 -----------------------------------------------------------
core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java Line 1766 (original), 1766 (patched) <https://reviews.apache.org/r/66243/#comment280381> I don't really get the difference between `jobConf` and `context.getProtoActionConf`, but I think it's important. Would you mind having some comments / JUnit examples for real contents? core/src/test/java/org/apache/oozie/action/hadoop/LauncherMainTester.java Lines 107-120 (patched) <https://reviews.apache.org/r/66243/#comment280363> Extract method `createSleepMapperReducerJobConf()`. core/src/test/java/org/apache/oozie/action/hadoop/LauncherMainTester.java Lines 129 (patched) <https://reviews.apache.org/r/66243/#comment280364> Some message to standard error? core/src/test/java/org/apache/oozie/action/hadoop/LauncherMainTester.java Lines 137 (patched) <https://reviews.apache.org/r/66243/#comment280366> Change signature to `writeJobID(Path, JobConf, String)`. core/src/test/java/org/apache/oozie/action/hadoop/LauncherMainTester.java Lines 138-142 (patched) <https://reviews.apache.org/r/66243/#comment280365> Use try-with-resources for `FileSystem` and `OutputStreamWriter`. core/src/test/java/org/apache/oozie/action/hadoop/LauncherMainTester.java Lines 145-152 (patched) <https://reviews.apache.org/r/66243/#comment280367> Use instead `writeJobID(Path, JobConf, String)`. core/src/test/java/org/apache/oozie/action/hadoop/SleepMapperReducerForTest.java Lines 35 (patched) <https://reviews.apache.org/r/66243/#comment280368> `SLEEP_TIME_MILLIS` core/src/test/java/org/apache/oozie/action/hadoop/SleepMapperReducerForTest.java Lines 46-50 (patched) <https://reviews.apache.org/r/66243/#comment280369> Extract method `sleepUninterrupted(long millis, String message)`. core/src/test/java/org/apache/oozie/action/hadoop/SleepMapperReducerForTest.java Lines 56-60 (patched) <https://reviews.apache.org/r/66243/#comment280370> Use instead method `sleepUninterrupted(long millis, String message)`. core/src/test/java/org/apache/oozie/action/oozie/TestSubWorkflowActionExecutor.java Line 40 (original), 58 (patched) <https://reviews.apache.org/r/66243/#comment280371> Remove `*` import. core/src/test/java/org/apache/oozie/action/oozie/TestSubWorkflowActionExecutor.java Lines 542-543 (patched) <https://reviews.apache.org/r/66243/#comment280372> Extract method `killWorkflow(String id)` and remove comment. core/src/test/java/org/apache/oozie/action/oozie/TestSubWorkflowActionExecutor.java Lines 545-546 (patched) <https://reviews.apache.org/r/66243/#comment280373> Rename `childMRJobApplicationId` to `externalChildJobId` and remove comment. core/src/test/java/org/apache/oozie/action/oozie/TestSubWorkflowActionExecutor.java Lines 554 (patched) <https://reviews.apache.org/r/66243/#comment280376> Cannot decrypt `sack` :( core/src/test/java/org/apache/oozie/action/oozie/TestSubWorkflowActionExecutor.java Lines 560 (patched) <https://reviews.apache.org/r/66243/#comment280377> Why not wait for `JOB_TIMEOUT`? core/src/test/java/org/apache/oozie/action/oozie/TestSubWorkflowActionExecutor.java Lines 567 (patched) <https://reviews.apache.org/r/66243/#comment280375> Use `try-with-resources`. core/src/test/java/org/apache/oozie/action/oozie/TestSubWorkflowActionExecutor.java Lines 575-576 (patched) <https://reviews.apache.org/r/66243/#comment280374> Rename `ex` to `ignored` and remove comment. core/src/test/java/org/apache/oozie/action/oozie/TestSubWorkflowActionExecutor.java Lines 596-597 (patched) <https://reviews.apache.org/r/66243/#comment280379> Extract local variables `isSubWfRunning` and `isSubWfActionRunning`. core/src/test/java/org/apache/oozie/action/oozie/TestSubWorkflowActionExecutor.java Lines 597 (patched) <https://reviews.apache.org/r/66243/#comment280378> Use `getSubWFId()`. core/src/test/java/org/apache/oozie/action/oozie/TestSubWorkflowActionExecutor.java Lines 717-719 (patched) <https://reviews.apache.org/r/66243/#comment280380> Use `try-with-resources`. src/main/resources/checkstyle.xml Lines 29-32 (original) <https://reviews.apache.org/r/66243/#comment280382> Submit another patch for another JIRA, please. - András Piros On March 23, 2018, 10:50 a.m., Peter Cseh wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/66243/ > ----------------------------------------------------------- > > (Updated March 23, 2018, 10:50 a.m.) > > > Review request for oozie and András Piros. > > > Bugs: OOZIE-3193 > https://issues.apache.org/jira/browse/OOZIE-3193 > > > Repository: oozie-git > > > Description > ------- > > Tags were not generated properly for external child jobs launched from subWFs > and they were not killed. > > > Diffs > ----- > > core/src/main/java/org/apache/oozie/action/ActionExecutor.java > 919509d35aefae8eed60849ca9c1da4e7ea291de > core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java > 122dfd02e17456b9aab924504faee790813fb6a1 > core/src/test/java/org/apache/oozie/action/hadoop/LauncherMainTester.java > fdca5706efbc13741477cee186550d07eee9f737 > > core/src/test/java/org/apache/oozie/action/hadoop/SleepMapperReducerForTest.java > PRE-CREATION > > core/src/test/java/org/apache/oozie/action/oozie/TestSubWorkflowActionExecutor.java > e074d482fb520e96de01b605ee8949ea52fd0d84 > > > Diff: https://reviews.apache.org/r/66243/diff/2/ > > > Testing > ------- > > Tested on deployed cluster. > Added Junit test. > > > Thanks, > > Peter Cseh > >
