> On June 7, 2013, 8:47 p.m., Rohini Palaniswamy wrote:
> > trunk/core/src/main/java/org/apache/oozie/action/hadoop/MapReduceActionExecutor.java,
> >  line 141
> > <https://reviews.apache.org/r/11680/diff/1/?file=301404#file301404line141>
> >
> >     Does core now depend on sharelib/oozie?

Ya, it got kinda tricky.  

- Core depends on Oozie sharelib as compile
- Pig/Hive/etc (except for Oozie) sharelibs depends on Oozie sharelib and Core 
as provided
- Webapp depends on Core, Pig/Hive/Oozie/etc sharelibs as compile


> On June 7, 2013, 8:47 p.m., Rohini Palaniswamy wrote:
> > trunk/core/src/main/resources/oozie-default.xml, line 31
> > <https://reviews.apache.org/r/11680/diff/2/?file=301866#file301866line31>
> >
> >     TODO needs to be replaced with content

Sorry, I completely forgot about that; I'll add a description.  I also forgot 
to add something about it to the documentation.


> On June 7, 2013, 8:47 p.m., Rohini Palaniswamy wrote:
> > trunk/core/src/main/resources/oozie-default.xml, line 28
> > <https://reviews.apache.org/r/11680/diff/2/?file=301866#file301866line28>
> >
> >     Can we rename it to something like action.ship.launcher.jar which makes 
> > it more clear?

Sure, "oozie.action.ship.launcher.jar" sounds good to me.


> On June 7, 2013, 8:47 p.m., Rohini Palaniswamy wrote:
> > trunk/core/src/main/java/org/apache/oozie/action/hadoop/LauncherMapperHelper.java,
> >  line 50
> > <https://reviews.apache.org/r/11680/diff/1/?file=301400#file301400line50>
> >
> >     empty return. 
> >     
> >     Can we fix/add javadoc for methods in this class or else altogether 
> > remove them. Few of them just have generated java doc without anything in 
> > them.

I'll just remove them for now; if we want to have javadocs for them, then I 
think that can be a separate JIRA (in fact, it may make sense to have a JIRA to 
go through and add Javadoc for all public-facing methods?)


> On June 7, 2013, 8:47 p.m., Rohini Palaniswamy wrote:
> > trunk/core/src/main/java/org/apache/oozie/action/hadoop/HiveActionExecutor.java,
> >  line 62
> > <https://reviews.apache.org/r/11680/diff/1/?file=301395#file301395line62>
> >
> >     Can we can now throw RunTimeException here? Same for other action 
> > executors like pig.

That should be fine for a deployment, but during the tests it was causing many 
to fail because they load the services, including ActionService, which then 
tries to initialize all of the ActionExecutors, which then tries to create the 
launcher jar by finding the Main classes, which are not available to core 
during testing.  

I agree that it would be better to throw a RuntimeException: I'll look into 
this some more; perhaps I can tweak a few tests to make it work.


- Robert


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


On June 7, 2013, 8:21 p.m., Robert Kanter wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/11680/
> -----------------------------------------------------------
> 
> (Updated June 7, 2013, 8:21 p.m.)
> 
> 
> Review request for oozie.
> 
> 
> Description
> -------
> 
> The patch is mostly moving some files from core to the Oozie sharelib. There 
> was some minor refactoring to fix some issues with tests. Otherwise, the only 
> actual code changes was to add the option to use the launcher jar or not 
> (oozie.action.use.launcher.jar).
> 
> 
> This addresses bug OOZIE-1315.
>     https://issues.apache.org/jira/browse/OOZIE-1315
> 
> 
> Diffs
> -----
> 
>   trunk/core/pom.xml 1490336 
>   trunk/core/src/main/java/org/apache/oozie/action/hadoop/ActionStats.java 
> 1490336 
>   trunk/core/src/main/java/org/apache/oozie/action/hadoop/ActionType.java 
> 1490336 
>   
> trunk/core/src/main/java/org/apache/oozie/action/hadoop/FSLauncherURIHandler.java
>  1490336 
>   
> trunk/core/src/main/java/org/apache/oozie/action/hadoop/HCatLauncherURIHandler.java
>  1490336 
>   
> trunk/core/src/main/java/org/apache/oozie/action/hadoop/HiveActionExecutor.java
>  1490336 
>   
> trunk/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java
>  1490336 
>   
> trunk/core/src/main/java/org/apache/oozie/action/hadoop/LauncherException.java
>  1490336 
>   trunk/core/src/main/java/org/apache/oozie/action/hadoop/LauncherMain.java 
> 1490336 
>   trunk/core/src/main/java/org/apache/oozie/action/hadoop/LauncherMapper.java 
> 1490336 
>   
> trunk/core/src/main/java/org/apache/oozie/action/hadoop/LauncherMapperHelper.java
>  PRE-CREATION 
>   
> trunk/core/src/main/java/org/apache/oozie/action/hadoop/LauncherURIHandler.java
>  1490336 
>   
> trunk/core/src/main/java/org/apache/oozie/action/hadoop/LauncherURIHandlerFactory.java
>  1490336 
>   trunk/core/src/main/java/org/apache/oozie/action/hadoop/MRStats.java 
> 1490336 
>   
> trunk/core/src/main/java/org/apache/oozie/action/hadoop/MapReduceActionExecutor.java
>  1490336 
>   trunk/core/src/main/java/org/apache/oozie/action/hadoop/MapReduceMain.java 
> 1490336 
>   
> trunk/core/src/main/java/org/apache/oozie/action/hadoop/PigActionExecutor.java
>  1490336 
>   trunk/core/src/main/java/org/apache/oozie/action/hadoop/PipesMain.java 
> 1490336 
>   
> trunk/core/src/main/java/org/apache/oozie/action/hadoop/PrepareActionsDriver.java
>  1490336 
>   trunk/core/src/main/java/org/apache/oozie/action/hadoop/ShellMain.java 
> 1490336 
>   
> trunk/core/src/main/java/org/apache/oozie/action/hadoop/SqoopActionExecutor.java
>  1490336 
>   trunk/core/src/main/java/org/apache/oozie/util/HCatURI.java 1490336 
>   trunk/core/src/main/resources/oozie-default.xml 1490336 
>   
> trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestFSPrepareActions.java
>  1490336 
>   
> trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestHCatPrepareActions.java
>  1490336 
>   
> trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestJavaActionExecutor.java
>  1490336 
>   trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestLauncher.java 
> 1490336 
>   
> trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestMapReduceActionError.java
>  1490336 
>   
> trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestMapReduceActionExecutor.java
>  1490336 
>   
> trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestMapReduceActionExecutorUberJar.java
>  1490336 
>   
> trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestPrepareActionsDriver.java
>  1490336 
>   
> trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestShellActionExecutor.java
>  1490336 
>   
> trunk/core/src/test/java/org/apache/oozie/command/wf/TestActionCheckXCommand.java
>  1490336 
>   
> trunk/core/src/test/java/org/apache/oozie/command/wf/TestActionStartXCommand.java
>  1490336 
>   trunk/core/src/test/java/org/apache/oozie/service/TestRecoveryService.java 
> 1490336 
>   trunk/core/src/test/java/org/apache/oozie/util/TestHCatURI.java 1490336 
>   trunk/core/src/test/resources/hsqldb-oozie-site.xml 1490336 
>   trunk/examples/pom.xml 1490336 
>   trunk/examples/src/test/resources/hsqldb-oozie-site.xml 1490336 
>   trunk/minitest/src/test/resources/hsqldb-oozie-site.xml 1490336 
>   trunk/pom.xml 1490336 
>   trunk/sharelib/distcp/pom.xml 1490336 
>   trunk/sharelib/hcatalog/pom.xml 1490336 
>   trunk/sharelib/hive/pom.xml 1490336 
>   
> trunk/sharelib/hive/src/test/java/org/apache/oozie/action/hadoop/TestHiveActionExecutor.java
>  1490336 
>   trunk/sharelib/oozie/pom.xml 1490336 
>   
> trunk/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherMapper.java
>  PRE-CREATION 
>   
> trunk/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/MapReduceMain.java
>  PRE-CREATION 
>   trunk/sharelib/pig/pom.xml 1490336 
>   
> trunk/sharelib/pig/src/test/java/org/apache/oozie/action/hadoop/TestPigActionExecutor.java
>  1490336 
>   trunk/sharelib/sqoop/pom.xml 1490336 
>   
> trunk/sharelib/sqoop/src/test/java/org/apache/oozie/action/hadoop/TestSqoopActionExecutor.java
>  1490336 
>   trunk/sharelib/streaming/pom.xml 1490336 
>   trunk/tools/src/test/resources/hsqldb-oozie-site.xml 1490336 
>   trunk/webapp/pom.xml 1490336 
> 
> Diff: https://reviews.apache.org/r/11680/diff/
> 
> 
> Testing
> -------
> 
> - Unit tests for oozie.action.use.launcher.jar
> - I verified that it works correctly with oozie.action.use.launcher.jar set 
> to true and false.
> 
> 
> Thanks,
> 
> Robert Kanter
> 
>

Reply via email to