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




core/src/main/java/org/apache/oozie/action/hadoop/LauncherConfigurationInjector.java
Lines 32-61 (patched)
<https://reviews.apache.org/r/62936/#comment265306>

    Writing examples to all items in the list to make the point more clear.



core/src/main/java/org/apache/oozie/action/hadoop/LauncherConfigurationInjector.java
Lines 95-96 (patched)
<https://reviews.apache.org/r/62936/#comment265308>

    I'm really rather in favor of making code more readable by separating 
concerns, using descriptive names, and having reasonable unit tests. In fact 
unit tests help me a lot usually when reading code.
    
    Putting examples (or even, Javadoc) on `private` methods that may change at 
any gives point of time seems to be very counterproductive to me. If you like, 
I can extract this (or any other) functionality to other classes and make 
separate unit tests for these.


- András Piros


On Oct. 16, 2017, 4:19 p.m., András Piros wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62936/
> -----------------------------------------------------------
> 
> (Updated Oct. 16, 2017, 4:19 p.m.)
> 
> 
> Review request for oozie, Attila Sasvari, Peter Cseh, and Peter Bacsko.
> 
> 
> Repository: oozie-git
> 
> 
> Description
> -------
> 
> OOZIE-2896 Ensure compatibility for existing LauncherMapper settings
> 
> 
> Diffs
> -----
> 
>   core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java 
> 76d0daa4f7528de6fb3c8993bae4dd28b6f7865b 
>   
> core/src/main/java/org/apache/oozie/action/hadoop/LauncherConfigurationInjector.java
>  PRE-CREATION 
>   core/src/main/java/org/apache/oozie/service/ConfigurationService.java 
> a51933022c14ea4ce06a65cc3c123bed8a8f7f34 
>   core/src/main/resources/oozie-default.xml 
> 2389b993b7b1769521d6dd0c7386e99e7516210a 
>   
> core/src/test/java/org/apache/oozie/action/hadoop/TestJavaActionExecutor.java 
> a7bd357f8938d148a150db43135a7a8fe7f0da4c 
>   
> core/src/test/java/org/apache/oozie/action/hadoop/TestLauncherConfigurationInjector.java
>  PRE-CREATION 
>   sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherAM.java 
> ee1a32ac9b071009e1dbe78396520d778f58bddc 
> 
> 
> Diff: https://reviews.apache.org/r/62936/diff/2/
> 
> 
> Testing
> -------
> 
> JUnit tests:
> 
> * `TestJavaActionExecutor`
> * `TestLauncherConfigurationFilter`
> 
> Functional tests: submit and run `examples/apps/java-main/workflow.xml` in 
> three modes:
> 
> * only `oozie.launcher.` parameters are present: `oozie.launcher.` parameters 
> are passed through
> * `oozie.launcher.` and override (`yarn.`, `mapreduce.`, and `mapred.`) 
> parameters are also present: `oozie.launcher.` parameters are passed through
> * only override (`yarn.`, `mapreduce.`, and `mapred.`) parameters are 
> present: override parameters are passed through
> 
> 
> Thanks,
> 
> András Piros
> 
>

Reply via email to