-----------------------------------------------------------
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)
<https://reviews.apache.org/r/68237/#comment292333>

    "NewYARN"



core/src/main/java/org/apache/oozie/action/hadoop/MapReduceActionExecutor.java
Lines 576-581 (patched)
<https://reviews.apache.org/r/68237/#comment292331>

    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)
<https://reviews.apache.org/r/68237/#comment292334>

    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)
<https://reviews.apache.org/r/68237/#comment292336>

    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)
<https://reviews.apache.org/r/68237/#comment292335>

    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)
<https://reviews.apache.org/r/68237/#comment292337>

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

Reply via email to