Re: Review Request 68237: OOZIE-3298 OYA: external ID is not filled properly and failing MR job is treated as SUCCEEDED

[András Piros via Review Board]

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

(Updated Sept. 10, 2018, 3:37 p.m.)


Review request for oozie, Andras Salamon, Attila Sasvari, Kinga Marton, and 
Peter Bacsko.


Changes
---

Addressing last FindBugs error.


Repository: oozie-git


Description
---

OOZIE-3298 OYA: external ID is not filled properly and failing MR job is 
treated as SUCCEEDED


Diffs (updated)
-

  core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java 
05fac3953eed5a5e2a16fa362656596f3a962a88 
  
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/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherMain.java 
c9e2a914a4b88640ac1c370b8971355dc087a235 
  
sharelib/streaming/src/test/java/org/apache/oozie/action/hadoop/TestMapReduceActionExecutor.java
 f460b6bd11f60dfb397c6bba82be1427c2d1b570 


Diff: https://reviews.apache.org/r/68237/diff/11/

Changes: https://reviews.apache.org/r/68237/diff/10-11/


Testing
---

Tested on a real cluster, plus added test cases to 
`TestMapReduceActionExecutor` and new test classes.


Thanks,

András Piros

Re: Review Request 68237: OOZIE-3298 OYA: external ID is not filled properly and failing MR job is treated as SUCCEEDED

[Peter Bacsko via Review Board]

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


Ship it!




Ship It!

- Peter Bacsko


On szept. 7, 2018, 8:08 de, András Piros wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68237/
> ---
> 
> (Updated szept. 7, 2018, 8:08 de)
> 
> 
> 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 
> 05fac3953eed5a5e2a16fa362656596f3a962a88 
>   
> 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/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherMain.java 
> c9e2a914a4b88640ac1c370b8971355dc087a235 
>   
> sharelib/streaming/src/test/java/org/apache/oozie/action/hadoop/TestMapReduceActionExecutor.java
>  f460b6bd11f60dfb397c6bba82be1427c2d1b570 
> 
> 
> Diff: https://reviews.apache.org/r/68237/diff/10/
> 
> 
> Testing
> ---
> 
> Tested on a real cluster, plus added test cases to 
> `TestMapReduceActionExecutor` and new test classes.
> 
> 
> Thanks,
> 
> András Piros
> 
>

Re: Review Request 68237: OOZIE-3298 OYA: external ID is not filled properly and failing MR job is treated as SUCCEEDED

[András Piros via Review Board]

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

(Updated Sept. 7, 2018, 8:08 a.m.)


Review request for oozie, Andras Salamon, Attila Sasvari, Kinga Marton, and 
Peter Bacsko.


Changes
---

Addressing review comments.


Repository: oozie-git


Description
---

OOZIE-3298 OYA: external ID is not filled properly and failing MR job is 
treated as SUCCEEDED


Diffs (updated)
-

  core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java 
05fac3953eed5a5e2a16fa362656596f3a962a88 
  
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/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherMain.java 
c9e2a914a4b88640ac1c370b8971355dc087a235 
  
sharelib/streaming/src/test/java/org/apache/oozie/action/hadoop/TestMapReduceActionExecutor.java
 f460b6bd11f60dfb397c6bba82be1427c2d1b570 


Diff: https://reviews.apache.org/r/68237/diff/10/

Changes: https://reviews.apache.org/r/68237/diff/9-10/


Testing
---

Tested on a real cluster, plus added test cases to 
`TestMapReduceActionExecutor` and new test classes.


Thanks,

András Piros

Re: Review Request 68237: OOZIE-3298 OYA: external ID is not filled properly and failing MR job is treated as SUCCEEDED

[András Piros via Review Board]

> On Sept. 6, 2018, 9:42 p.m., Peter Bacsko wrote:
> > core/src/test/java/org/apache/oozie/action/hadoop/ActionExecutorTestCase.java
> > Lines 377 (patched)
> > 
> >
> > What's the reason for the list? Couldn't it be just an ApplicationId 
> > reference?

Moved from 
[`TestSubWorkflowActionExecutor`](https://github.com/apache/oozie/blob/master/core/src/test/java/org/apache/oozie/action/oozie/TestSubWorkflowActionExecutor.java#L547-L568)
 for code reuse.


> On Sept. 6, 2018, 9:42 p.m., Peter Bacsko wrote:
> > core/src/test/java/org/apache/oozie/action/hadoop/ActionExecutorTestCase.java
> > Lines 388 (patched)
> > 
> >
> > Does that file contains only a single line? If so, this piece of code 
> > can be simplified.

Moved from 
[`TestSubWorkflowActionExecutor`](https://github.com/apache/oozie/blob/master/core/src/test/java/org/apache/oozie/action/oozie/TestSubWorkflowActionExecutor.java#L547-L568)
 for code reuse.


> On Sept. 6, 2018, 9:42 p.m., Peter Bacsko wrote:
> > core/src/test/java/org/apache/oozie/action/hadoop/TestYarnApplicationIdFinder.java
> > Lines 43 (patched)
> > 
> >
> > Interesting... don't you need to add 
> > @RunWith(MockitoJunitRunner.class)? Does JUnit4 automatically handle @Mock 
> > members?

Indeed, instead of explicit `Mockito#mock()` calls now using 
`@RunWith(MockitoJUnitRunner.class)`.


- András


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


On Sept. 6, 2018, 3:59 p.m., András Piros wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68237/
> ---
> 
> (Updated Sept. 6, 2018, 3:59 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 
> 05fac3953eed5a5e2a16fa362656596f3a962a88 
>   
> 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/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherMain.java 
> c9e2a914a4b88640ac1c370b8971355dc087a235 
>   
> sharelib/streaming/src/test/java/org/apache/oozie/action/hadoop/TestMapReduceActionExecutor.java
>  f460b6bd11f60dfb397c6bba82be1427c2d1b570 
> 
> 
> Diff: https://reviews.apache.org/r/68237/diff/9/
> 
> 
> Testing
> ---
> 
> Tested on a real cluster, plus added test cases to 
> `TestMapReduceActionExecutor` and new test classes.
> 
> 
> Thanks,
> 
> András Piros
> 
>

Re: Review Request 68237: OOZIE-3298 OYA: external ID is not filled properly and failing MR job is treated as SUCCEEDED

[András Piros via Review Board]

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




core/src/main/java/org/apache/oozie/action/hadoop/MapReduceActionExecutor.java
Lines 576-581 (patched)


On the one hand we have an unordered `List` but in the 
reality we need only a `String` of the biggest `ApplicationId`.

Extracted method and written Javadoc to make the purpose clearer. Used 
`Iterables#transform()` and `Ordering#max()` for clarity.



core/src/main/java/org/apache/oozie/action/hadoop/MapReduceActionExecutor.java
Lines 647-657 (patched)


Yes. Added Javadoc to make it clearer.


- András Piros


On Sept. 6, 2018, 3:59 p.m., András Piros wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68237/
> ---
> 
> (Updated Sept. 6, 2018, 3:59 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 
> 05fac3953eed5a5e2a16fa362656596f3a962a88 
>   
> 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/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherMain.java 
> c9e2a914a4b88640ac1c370b8971355dc087a235 
>   
> sharelib/streaming/src/test/java/org/apache/oozie/action/hadoop/TestMapReduceActionExecutor.java
>  f460b6bd11f60dfb397c6bba82be1427c2d1b570 
> 
> 
> Diff: https://reviews.apache.org/r/68237/diff/9/
> 
> 
> Testing
> ---
> 
> Tested on a real cluster, plus added test cases to 
> `TestMapReduceActionExecutor` and new test classes.
> 
> 
> Thanks,
> 
> András Piros
> 
>

Re: Review Request 68237: OOZIE-3298 OYA: external ID is not filled properly and failing MR job is treated as SUCCEEDED

[Peter Bacsko via Review Board]

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




core/src/main/java/org/apache/oozie/action/hadoop/MapReduceActionExecutor.java
Lines 541 (patched)


"NewYARN"



core/src/main/java/org/apache/oozie/action/hadoop/MapReduceActionExecutor.java
Lines 576-581 (patched)


This looks very complicated. Can't we just use Collections.sort() with 
passing the comparator?



core/src/main/java/org/apache/oozie/action/hadoop/MapReduceActionExecutor.java
Lines 647-657 (patched)


This is a separate class so that the Reader can be mocked in the tests, 
right?



core/src/test/java/org/apache/oozie/action/hadoop/ActionExecutorTestCase.java
Lines 377 (patched)


What's the reason for the list? Couldn't it be just an ApplicationId 
reference?



core/src/test/java/org/apache/oozie/action/hadoop/ActionExecutorTestCase.java
Lines 388 (patched)


Does that file contains only a single line? If so, this piece of code can 
be simplified.



core/src/test/java/org/apache/oozie/action/hadoop/TestYarnApplicationIdFinder.java
Lines 43 (patched)


Interesting... don't you need to add @RunWith(MockitoJunitRunner.class)? 
Does JUnit4 automatically handle @Mock members?


- Peter Bacsko


On szept. 6, 2018, 3:59 du, András Piros wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68237/
> ---
> 
> (Updated szept. 6, 2018, 3:59 du)
> 
> 
> 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 
> 05fac3953eed5a5e2a16fa362656596f3a962a88 
>   
> 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/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherMain.java 
> c9e2a914a4b88640ac1c370b8971355dc087a235 
>   
> sharelib/streaming/src/test/java/org/apache/oozie/action/hadoop/TestMapReduceActionExecutor.java
>  f460b6bd11f60dfb397c6bba82be1427c2d1b570 
> 
> 
> Diff: https://reviews.apache.org/r/68237/diff/9/
> 
> 
> Testing
> ---
> 
> Tested on a real cluster, plus added test cases to 
> `TestMapReduceActionExecutor` and new test classes.
> 
> 
> Thanks,
> 
> András Piros
> 
>

Re: Review Request 68237: OOZIE-3298 OYA: external ID is not filled properly and failing MR job is treated as SUCCEEDED

[András Piros via Review Board]

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

(Updated Sept. 6, 2018, 3:59 p.m.)


Review request for oozie, Andras Salamon, Attila Sasvari, Kinga Marton, and 
Peter Bacsko.


Changes
---

Changes:

* the class `YarnApplicationIdComparator` now considers also `ApplicationId` 
instances with different cluster timestamps as different
* refactored 
`MapReduceActionExecutor$YarnApplicationIdFinder#fallbackToYarnChildOrExternalId()`
 not to call `YarnClient#getApplications()` directly because of performance 
issues on a busy cluster. Instead, call 
`LauncherMain#getChildYarnApplications()` that calls 
`ClientRMProxy#getApplications(GetApplicationRequest request)` instead, for 
better filtering options. This way, performance doesn't suffer even on busy 
YARN clusters


Repository: oozie-git


Description
---

OOZIE-3298 OYA: external ID is not filled properly and failing MR job is 
treated as SUCCEEDED


Diffs (updated)
-

  core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java 
05fac3953eed5a5e2a16fa362656596f3a962a88 
  
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/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherMain.java 
c9e2a914a4b88640ac1c370b8971355dc087a235 
  
sharelib/streaming/src/test/java/org/apache/oozie/action/hadoop/TestMapReduceActionExecutor.java
 f460b6bd11f60dfb397c6bba82be1427c2d1b570 


Diff: https://reviews.apache.org/r/68237/diff/9/

Changes: https://reviews.apache.org/r/68237/diff/8-9/


Testing
---

Tested on a real cluster, plus added test cases to 
`TestMapReduceActionExecutor` and new test classes.


Thanks,

András Piros

Re: Review Request 68237: OOZIE-3298 OYA: external ID is not filled properly and failing MR job is treated as SUCCEEDED

[András Piros via Review Board]

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

(Updated Sept. 3, 2018, 9:10 a.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/8/


Testing (updated)
---

Tested on a real cluster, plus added test cases to 
`TestMapReduceActionExecutor` and new test classes.


Thanks,

András Piros

Re: Review Request 68237: OOZIE-3298 OYA: external ID is not filled properly and failing MR job is treated as SUCCEEDED

[András Piros via Review Board]

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

(Updated Sept. 3, 2018, 7:59 a.m.)


Review request for oozie, Andras Salamon, Attila Sasvari, Kinga Marton, and 
Peter Bacsko.


Changes
---

More comments on `MapReduceActionExecutor` and `TestMapReduceActionExecutor` 
classes.


Repository: oozie-git


Description
---

OOZIE-3298 OYA: external ID is not filled properly and failing MR job is 
treated as SUCCEEDED


Diffs (updated)
-

  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/8/

Changes: https://reviews.apache.org/r/68237/diff/7-8/


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

Re: Review Request 68237: OOZIE-3298 OYA: external ID is not filled properly and failing MR job is treated as SUCCEEDED

[András Piros via Review Board]

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

(Updated Aug. 31, 2018, 10:50 a.m.)


Review request for oozie, Andras Salamon, Attila Sasvari, Kinga Marton, and 
Peter Bacsko.


Changes
---

Addressing review comments.


Repository: oozie-git


Description
---

OOZIE-3298 OYA: external ID is not filled properly and failing MR job is 
treated as SUCCEEDED


Diffs (updated)
-

  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/7/

Changes: https://reviews.apache.org/r/68237/diff/6-7/


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

Re: Review Request 68237: OOZIE-3298 OYA: external ID is not filled properly and failing MR job is treated as SUCCEEDED

[András Piros via Review Board]

> On Aug. 30, 2018, 1:34 p.m., Peter Bacsko wrote:
> > core/src/main/java/org/apache/oozie/action/hadoop/MapReduceActionExecutor.java
> > Lines 575 (patched)
> > 
> >
> > Can't we just compare the IDs as strings? What's the rationale of this 
> > class?
> > 
> > If it's not passed to other data structures or classes which use a 
> > Comparator, then I guess it doesn't have to implement it.

The rationale behind this class is to compare two YARN application ID 
`String`s, when the cluster startup time is different. Newer application IDs 
will be treated as "greater".

We cannot really use plain `String` comparison because we could face situations 
where different application IDs are read with different cluster startup times.


- András


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


On Aug. 24, 2018, 2:18 p.m., András Piros wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68237/
> ---
> 
> (Updated Aug. 24, 2018, 2:18 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/6/
> 
> 
> 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
> 
>

Re: Review Request 68237: OOZIE-3298 OYA: external ID is not filled properly and failing MR job is treated as SUCCEEDED

[Peter Bacsko via Review Board]

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




core/src/main/java/org/apache/oozie/action/hadoop/MapReduceActionExecutor.java
Lines 448 (patched)


Complete javadoc



core/src/main/java/org/apache/oozie/action/hadoop/MapReduceActionExecutor.java
Lines 451 (patched)


Add @Override



core/src/main/java/org/apache/oozie/action/hadoop/MapReduceActionExecutor.java
Lines 575 (patched)


Can't we just compare the IDs as strings? What's the rationale of this 
class?

If it's not passed to other data structures or classes which use a 
Comparator, then I guess it doesn't have to implement it.



core/src/main/java/org/apache/oozie/action/hadoop/MapReduceActionExecutor.java
Lines 576 (patched)


Is Serializable necessary here to implement?

If Findbugs complains about it, just add the appropriate annotation to 
suppress it (and add a short justification).



core/src/test/java/org/apache/oozie/action/hadoop/ActionExecutorTestCase.java
Lines 348 (patched)


Remove this



sharelib/streaming/src/test/java/org/apache/oozie/action/hadoop/TestMapReduceActionExecutor.java
Lines 1385 (patched)


What is this part doing? Could you add comments please? It's not evident.


- Peter Bacsko


On aug. 24, 2018, 2:18 du, András Piros wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68237/
> ---
> 
> (Updated aug. 24, 2018, 2:18 du)
> 
> 
> 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/6/
> 
> 
> 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
> 
>

Re: Review Request 68237: OOZIE-3298 OYA: external ID is not filled properly and failing MR job is treated as SUCCEEDED

[András Piros via Review Board]

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

(Updated Aug. 24, 2018, 2:18 p.m.)


Review request for oozie, Andras Salamon, Attila Sasvari, Kinga Marton, and 
Peter Bacsko.


Changes
---

Yet another review round.


Repository: oozie-git


Description
---

OOZIE-3298 OYA: external ID is not filled properly and failing MR job is 
treated as SUCCEEDED


Diffs (updated)
-

  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/6/

Changes: https://reviews.apache.org/r/68237/diff/5-6/


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

Re: Review Request 68237: OOZIE-3298 OYA: external ID is not filled properly and failing MR job is treated as SUCCEEDED

[Andras Salamon]

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




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


Two times we need oozie:launcher, one time we need oozie:action prefix, 
this methods needs an additional parameter.


- Andras Salamon


On Aug. 24, 2018, 12:17 p.m., András Piros wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/68237/
> ---
> 
> (Updated Aug. 24, 2018, 12:17 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/5/
> 
> 
> 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
> 
>

Re: Review Request 68237: OOZIE-3298 OYA: external ID is not filled properly and failing MR job is treated as SUCCEEDED

[András Piros via Review Board]

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

(Updated Aug. 24, 2018, 12:17 p.m.)


Review request for oozie, Andras Salamon, Attila Sasvari, Kinga Marton, and 
Peter Bacsko.


Changes
---

Addressing pre-commit build errors and Andras Salamon's review comments.


Repository: oozie-git


Description
---

OOZIE-3298 OYA: external ID is not filled properly and failing MR job is 
treated as SUCCEEDED


Diffs (updated)
-

  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/5/

Changes: https://reviews.apache.org/r/68237/diff/4-5/


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

Re: Review Request 68237: OOZIE-3298 OYA: external ID is not filled properly and failing MR job is treated as SUCCEEDED

[András Piros via Review Board]

> On Aug. 24, 2018, 8:02 a.m., Andras Salamon wrote:
> > sharelib/streaming/src/test/java/org/apache/oozie/action/hadoop/TestMapReduceActionExecutor.java
> > Lines 1389 (patched)
> > 
> >
> > extra space indentation

This is because it's within a `try-with-resources` statement declaration.


- András


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


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
> 
>

Re: Review Request 68237: OOZIE-3298 OYA: external ID is not filled properly and failing MR job is treated as SUCCEEDED

[Andras Salamon]

---
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)


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)


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)


Third occurance of the similar String formatting.



core/src/main/java/org/apache/oozie/action/hadoop/MapReduceActionExecutor.java
Line 24 (original), 24 (patched)


Please remove * import



core/src/main/java/org/apache/oozie/action/hadoop/MapReduceActionExecutor.java
Lines 26 (patched)


Please remove * import



core/src/main/java/org/apache/oozie/action/hadoop/MapReduceActionExecutor.java
Line 37 (original), 38 (patched)


Please remove * import



core/src/main/java/org/apache/oozie/action/hadoop/MapReduceActionExecutor.java
Lines 42 (patched)


Please remove * import



core/src/test/java/org/apache/oozie/action/hadoop/TestYarnApplicationIdComparator.java
Lines 45 (patched)


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_1_222 applicationId_3_444.



core/src/test/java/org/apache/oozie/action/hadoop/TestYarnApplicationIdComparator.java
Lines 47 (patched)


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)


extra space indentation



sharelib/streaming/src/test/java/org/apache/oozie/action/hadoop/TestMapReduceActionExecutor.java
Lines 1436 (patched)


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 
> 

Re: Review Request 68237: OOZIE-3298 OYA: external ID is not filled properly and failing MR job is treated as SUCCEEDED

[András Piros via Review Board]

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

(Updated Aug. 23, 2018, 6:35 p.m.)


Review request for oozie, Attila Sasvari 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 (updated)
-

  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/

Changes: https://reviews.apache.org/r/68237/diff/3-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

Re: Review Request 68237: OOZIE-3298 OYA: external ID is not filled properly and failing MR job is treated as SUCCEEDED

[András Piros via Review Board]

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

(Updated Aug. 13, 2018, 3:39 p.m.)


Review request for oozie, Attila Sasvari 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 (updated)
-

  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/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/3/

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


Testing
---

DO NOT MERGE YET.

Tested on a real cluster, plus added test cases to `TestJavaActionExecutor` and 
`TestMapReduceActionExecutor`.

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

Re: Review Request 68237: OOZIE-3298 OYA: external ID is not filled properly and failing MR job is treated as SUCCEEDED

[András Piros via Review Board]

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

(Updated Aug. 10, 2018, 10:52 a.m.)


Review request for oozie, Attila Sasvari 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 (updated)
-

  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/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/2/

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


Testing
---

DO NOT MERGE YET.

Tested on a real cluster, plus added test cases to `TestJavaActionExecutor` and 
`TestMapReduceActionExecutor`.

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