Re: Review Request 68102: OOZIE-3193 Applications are not killed when submitted via subworkflow

[András Piros via Review Board]

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


Ship it!




Ship It!

- András Piros


On Aug. 1, 2018, 1:44 p.m., Kinga Marton wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68102/
> ---
> 
> (Updated Aug. 1, 2018, 1:44 p.m.)
> 
> 
> Review request for oozie, András Piros and Peter Cseh.
> 
> 
> 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 71bd36ae6 
>   core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java 
> a1a9671ab 
>   core/src/test/java/org/apache/oozie/action/hadoop/LauncherMainTester.java 
> fdca5706e 
>   
> core/src/test/java/org/apache/oozie/action/hadoop/SleepMapperReducerForTest.java
>  PRE-CREATION 
>   
> core/src/test/java/org/apache/oozie/action/oozie/TestSubWorkflowActionExecutor.java
>  e074d482f 
> 
> 
> Diff: https://reviews.apache.org/r/68102/diff/3/
> 
> 
> Testing
> ---
> 
> Tested manually.
> Added Junit test.
> 
> 
> Thanks,
> 
> Kinga Marton
> 
>

Re: Review Request 68102: OOZIE-3193 Applications are not killed when submitted via subworkflow

[Kinga Marton via Review Board]

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

(Updated Aug. 1, 2018, 1:44 p.m.)


Review request for oozie, András Piros and Peter Cseh.


Repository: oozie-git


Description
---

Tags were not generated properly for external child jobs launched from subWFs 
and they were not killed.


Diffs (updated)
-

  core/src/main/java/org/apache/oozie/action/ActionExecutor.java 71bd36ae6 
  core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java 
a1a9671ab 
  core/src/test/java/org/apache/oozie/action/hadoop/LauncherMainTester.java 
fdca5706e 
  
core/src/test/java/org/apache/oozie/action/hadoop/SleepMapperReducerForTest.java
 PRE-CREATION 
  
core/src/test/java/org/apache/oozie/action/oozie/TestSubWorkflowActionExecutor.java
 e074d482f 


Diff: https://reviews.apache.org/r/68102/diff/3/

Changes: https://reviews.apache.org/r/68102/diff/2-3/


Testing
---

Tested manually.
Added Junit test.


Thanks,

Kinga Marton

Re: Review Request 68102: OOZIE-3193 Applications are not killed when submitted via subworkflow

[András Piros via Review Board]

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




core/src/test/java/org/apache/oozie/action/oozie/TestSubWorkflowActionExecutor.java
Lines 931-932 (patched)


Can be `final`.



core/src/test/java/org/apache/oozie/action/oozie/TestSubWorkflowActionExecutor.java
Lines 946-947 (patched)


Can be `final`.


- András Piros


On Aug. 1, 2018, 9:59 a.m., Kinga Marton wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68102/
> ---
> 
> (Updated Aug. 1, 2018, 9:59 a.m.)
> 
> 
> Review request for oozie, András Piros and Peter Cseh.
> 
> 
> 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 71bd36ae6 
>   core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java 
> a1a9671ab 
>   core/src/test/java/org/apache/oozie/action/hadoop/LauncherMainTester.java 
> fdca5706e 
>   
> core/src/test/java/org/apache/oozie/action/hadoop/SleepMapperReducerForTest.java
>  PRE-CREATION 
>   
> core/src/test/java/org/apache/oozie/action/oozie/TestSubWorkflowActionExecutor.java
>  e074d482f 
> 
> 
> Diff: https://reviews.apache.org/r/68102/diff/2/
> 
> 
> Testing
> ---
> 
> Tested manually.
> Added Junit test.
> 
> 
> Thanks,
> 
> Kinga Marton
> 
>

Re: Review Request 68102: OOZIE-3193 Applications are not killed when submitted via subworkflow

[András Piros via Review Board]

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


Ship it!




Ship It!

- András Piros


On Aug. 1, 2018, 9:59 a.m., Kinga Marton wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68102/
> ---
> 
> (Updated Aug. 1, 2018, 9:59 a.m.)
> 
> 
> Review request for oozie, András Piros and Peter Cseh.
> 
> 
> 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 71bd36ae6 
>   core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java 
> a1a9671ab 
>   core/src/test/java/org/apache/oozie/action/hadoop/LauncherMainTester.java 
> fdca5706e 
>   
> core/src/test/java/org/apache/oozie/action/hadoop/SleepMapperReducerForTest.java
>  PRE-CREATION 
>   
> core/src/test/java/org/apache/oozie/action/oozie/TestSubWorkflowActionExecutor.java
>  e074d482f 
> 
> 
> Diff: https://reviews.apache.org/r/68102/diff/2/
> 
> 
> Testing
> ---
> 
> Tested manually.
> Added Junit test.
> 
> 
> Thanks,
> 
> Kinga Marton
> 
>

Re: Review Request 68102: OOZIE-3193 Applications are not killed when submitted via subworkflow

[Kinga Marton via Review Board]

> On July 30, 2018, 10:46 a.m., András Piros wrote:
> > core/src/test/java/org/apache/oozie/action/hadoop/SleepMapperReducerForTest.java
> > Lines 60-61 (patched)
> > 
> >
> > Isn't emitting another message also just after sleep a good idea?

I have added a wake up message as well :)


> On July 30, 2018, 10:46 a.m., András Piros wrote:
> > core/src/test/java/org/apache/oozie/action/oozie/TestSubWorkflowActionExecutor.java
> > Lines 555-574 (patched)
> > 
> >
> > Please extract to `private static class ApplicationIdExistsPredicate 
> > implements Predicate`.

I have extracted only the part performing the check.


> On July 30, 2018, 10:46 a.m., András Piros wrote:
> > core/src/test/java/org/apache/oozie/action/oozie/TestSubWorkflowActionExecutor.java
> > Lines 708-712 (patched)
> > 
> >
> > Please use latest schema version `1.0`.
> 
> Peter Cseh wrote:
> I don't think we should give up test coverage for older schemas. We 
> should keep around the old ones in test and make sure to add enough covergave 
> for new schemas when we add them.

I can change here to the new schema, since there are some similar workflows 
using the old schema.


- Kinga


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


On Aug. 1, 2018, 9:59 a.m., Kinga Marton wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68102/
> ---
> 
> (Updated Aug. 1, 2018, 9:59 a.m.)
> 
> 
> Review request for oozie, András Piros and Peter Cseh.
> 
> 
> 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 71bd36ae6 
>   core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java 
> a1a9671ab 
>   core/src/test/java/org/apache/oozie/action/hadoop/LauncherMainTester.java 
> fdca5706e 
>   
> core/src/test/java/org/apache/oozie/action/hadoop/SleepMapperReducerForTest.java
>  PRE-CREATION 
>   
> core/src/test/java/org/apache/oozie/action/oozie/TestSubWorkflowActionExecutor.java
>  e074d482f 
> 
> 
> Diff: https://reviews.apache.org/r/68102/diff/2/
> 
> 
> Testing
> ---
> 
> Tested manually.
> Added Junit test.
> 
> 
> Thanks,
> 
> Kinga Marton
> 
>

Re: Review Request 68102: OOZIE-3193 Applications are not killed when submitted via subworkflow

[Kinga Marton via Review Board]

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

(Updated Aug. 1, 2018, 9:59 a.m.)


Review request for oozie, András Piros and Peter Cseh.


Repository: oozie-git


Description
---

Tags were not generated properly for external child jobs launched from subWFs 
and they were not killed.


Diffs (updated)
-

  core/src/main/java/org/apache/oozie/action/ActionExecutor.java 71bd36ae6 
  core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java 
a1a9671ab 
  core/src/test/java/org/apache/oozie/action/hadoop/LauncherMainTester.java 
fdca5706e 
  
core/src/test/java/org/apache/oozie/action/hadoop/SleepMapperReducerForTest.java
 PRE-CREATION 
  
core/src/test/java/org/apache/oozie/action/oozie/TestSubWorkflowActionExecutor.java
 e074d482f 


Diff: https://reviews.apache.org/r/68102/diff/2/

Changes: https://reviews.apache.org/r/68102/diff/1-2/


Testing
---

Tested manually.
Added Junit test.


Thanks,

Kinga Marton

Re: Review Request 68102: OOZIE-3193 Applications are not killed when submitted via subworkflow

[Peter Cseh via Review Board]

> On July 30, 2018, 10:46 a.m., András Piros wrote:
> > core/src/test/java/org/apache/oozie/action/oozie/TestSubWorkflowActionExecutor.java
> > Lines 708-712 (patched)
> > 
> >
> > Please use latest schema version `1.0`.

I don't think we should give up test coverage for older schemas. We should keep 
around the old ones in test and make sure to add enough covergave for new 
schemas when we add them.


- Peter


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


On July 30, 2018, 8:42 a.m., Kinga Marton wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68102/
> ---
> 
> (Updated July 30, 2018, 8:42 a.m.)
> 
> 
> Review request for oozie, András Piros and Peter Cseh.
> 
> 
> 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 71bd36ae6 
>   core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java 
> a1a9671ab 
>   core/src/test/java/org/apache/oozie/action/hadoop/LauncherMainTester.java 
> fdca5706e 
>   
> core/src/test/java/org/apache/oozie/action/hadoop/SleepMapperReducerForTest.java
>  PRE-CREATION 
>   
> core/src/test/java/org/apache/oozie/action/oozie/TestSubWorkflowActionExecutor.java
>  e074d482f 
> 
> 
> Diff: https://reviews.apache.org/r/68102/diff/1/
> 
> 
> Testing
> ---
> 
> Tested manually.
> Added Junit test.
> 
> 
> Thanks,
> 
> Kinga Marton
> 
>

Re: Review Request 68102: OOZIE-3193 Applications are not killed when submitted via subworkflow

[András Piros via Review Board]

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




core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java
Lines 1781-1782 (patched)


Please remove code comment and extract to a well-named method instead.



core/src/test/java/org/apache/oozie/action/hadoop/LauncherMainTester.java
Lines 106-121 (patched)


Extract method.



core/src/test/java/org/apache/oozie/action/hadoop/SleepMapperReducerForTest.java
Lines 32 (patched)


`SLEEP_TIME_MILLIS_KEY` is a better name.



core/src/test/java/org/apache/oozie/action/hadoop/SleepMapperReducerForTest.java
Lines 60-61 (patched)


Isn't emitting another message also just after sleep a good idea?



core/src/test/java/org/apache/oozie/action/oozie/TestSubWorkflowActionExecutor.java
Lines 544 (patched)


Please remove white space between method name and brackets.



core/src/test/java/org/apache/oozie/action/oozie/TestSubWorkflowActionExecutor.java
Lines 555-574 (patched)


Please extract to `private static class ApplicationIdExistsPredicate 
implements Predicate`.



core/src/test/java/org/apache/oozie/action/oozie/TestSubWorkflowActionExecutor.java
Lines 581-588 (patched)


Please extract to `private static class SubWorkflowActionRunningPredicate 
implements Predicate`.



core/src/test/java/org/apache/oozie/action/oozie/TestSubWorkflowActionExecutor.java
Lines 708-712 (patched)


Please use latest schema version `1.0`.


- András Piros


On July 30, 2018, 8:42 a.m., Kinga Marton wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68102/
> ---
> 
> (Updated July 30, 2018, 8:42 a.m.)
> 
> 
> Review request for oozie, András Piros and Peter Cseh.
> 
> 
> 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 71bd36ae6 
>   core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java 
> a1a9671ab 
>   core/src/test/java/org/apache/oozie/action/hadoop/LauncherMainTester.java 
> fdca5706e 
>   
> core/src/test/java/org/apache/oozie/action/hadoop/SleepMapperReducerForTest.java
>  PRE-CREATION 
>   
> core/src/test/java/org/apache/oozie/action/oozie/TestSubWorkflowActionExecutor.java
>  e074d482f 
> 
> 
> Diff: https://reviews.apache.org/r/68102/diff/1/
> 
> 
> Testing
> ---
> 
> Tested manually.
> Added Junit test.
> 
> 
> Thanks,
> 
> Kinga Marton
> 
>

Review Request 68102: OOZIE-3193 Applications are not killed when submitted via subworkflow

[Kinga Marton via Review Board]

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

Review request for oozie, András Piros and Peter Cseh.


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 71bd36ae6 
  core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java 
a1a9671ab 
  core/src/test/java/org/apache/oozie/action/hadoop/LauncherMainTester.java 
fdca5706e 
  
core/src/test/java/org/apache/oozie/action/hadoop/SleepMapperReducerForTest.java
 PRE-CREATION 
  
core/src/test/java/org/apache/oozie/action/oozie/TestSubWorkflowActionExecutor.java
 e074d482f 


Diff: https://reviews.apache.org/r/68102/diff/1/


Testing
---

Tested manually.
Added Junit test.


Thanks,

Kinga Marton