> On July 13, 2016, 9:01 a.m., Attila Sasvari wrote: > > core/src/test/java/org/apache/oozie/action/hadoop/TestJavaActionExecutor.java, > > line 371 > > <https://reviews.apache.org/r/49778/diff/2/?file=1443102#file1443102line371> > > > > ActionExecutorException, HadoopAccessorException, IOException can only > > be thrown
Looks like it's only ActionExecutorException. > On July 13, 2016, 9:01 a.m., Attila Sasvari wrote: > > sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/CallbackNotifier.java, > > line 116 > > <https://reviews.apache.org/r/49778/diff/2/?file=1443116#file1443116line116> > > > > network resources are not freed. connection should be closed after use This gets executed in the AM, which shoudl exit shortly after the callback is sent out, so exiting the JVM would close it anyway, but it's safer to explicetly close it, so I'll do that. > On July 13, 2016, 9:01 a.m., Attila Sasvari wrote: > > sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherAM.java, > > line 141 > > <https://reviews.apache.org/r/49778/diff/2/?file=1443117#file1443117line141> > > > > Multiple methods could extracted from this to make the method more > > readable. As I said on earlier comments, I'm not sure that it would be. Most of the code in there is error handling, and I think that splitting that between different methods will make it even more complicated and harder to read. > On July 13, 2016, 9:01 a.m., Attila Sasvari wrote: > > sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherAM.java, > > line 373 > > <https://reviews.apache.org/r/49778/diff/2/?file=1443117#file1443117line373> > > > > - resource leak in case of an IOException > > - is there any reason for not using BufferedReader? No real reason. I copied this from LauncherMapper, and was written with older stuff (like StringBuffer instead of StringBuilder). I'll fix this and the resource leak. - Robert ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/49778/#review142033 ----------------------------------------------------------- On July 13, 2016, 5:42 a.m., Robert Kanter wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/49778/ > ----------------------------------------------------------- > > (Updated July 13, 2016, 5:42 a.m.) > > > Review request for oozie. > > > Bugs: OOZIE-2590 > https://issues.apache.org/jira/browse/OOZIE-2590 > > > Repository: oozie-git > > > Description > ------- > > See OOZIE-2590 > > > Diffs > ----- > > core/pom.xml 6584af8123925e00f7b29d6b33201f4fc7dca231 > core/src/main/java/org/apache/oozie/action/hadoop/DistcpActionExecutor.java > 96726daa800861c15e4a0d97514fbd45e31fa000 > core/src/main/java/org/apache/oozie/action/hadoop/Hive2ActionExecutor.java > b5b1bf90891fde1b13ca058e0b5f538b1b683128 > core/src/main/java/org/apache/oozie/action/hadoop/HiveActionExecutor.java > c74e9e61c01a4b6430bbd5491c8cbaf0c4e90198 > core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java > 99e33445d68ac6a99a08e02ecc182f35103a53ce > core/src/main/java/org/apache/oozie/action/hadoop/LauncherMapperHelper.java > 69e1044e531a67c2d522f642376f4ad030423e53 > core/src/main/java/org/apache/oozie/action/hadoop/SparkActionExecutor.java > 252f461267818c0da91c7ad59380b2f18a074f9f > core/src/main/java/org/apache/oozie/action/hadoop/SqoopActionExecutor.java > 6813a37d0ffc66da41636fe01e9cc7a37d6f6e79 > core/src/main/java/org/apache/oozie/service/HadoopAccessorService.java > 794e8258472df5fe18299eb3c62471cf9fecc186 > core/src/main/java/org/apache/oozie/util/ClasspathUtils.java PRE-CREATION > core/src/main/resources/oozie-default.xml > 6c2f7d82e933614ea6b434f9b035dca3751d1c7f > core/src/test/java/org/apache/oozie/QueryServlet.java PRE-CREATION > core/src/test/java/org/apache/oozie/action/hadoop/TestCallbackNotifier.java > PRE-CREATION > > core/src/test/java/org/apache/oozie/action/hadoop/TestJavaActionExecutor.java > 879bfebf58acc12ce787b151a67eaaa0716ff2bb > core/src/test/java/org/apache/oozie/action/hadoop/TestLauncherAM.java > PRE-CREATION > > core/src/test/java/org/apache/oozie/action/hadoop/TestPrepareActionsDriver.java > df9e939cb236e8b65e7b3aa9ab683c33f5fa02b7 > > core/src/test/java/org/apache/oozie/action/hadoop/TestShellActionExecutor.java > 6a962a1030a16e61a2af27a6316c4d71b17ea5c9 > core/src/test/java/org/apache/oozie/command/wf/HangServlet.java > 3344cf97762c77f5660d94341e72df4243f76eb2 > core/src/test/java/org/apache/oozie/service/TestConfigurationService.java > 2153bf122ddbdf81acd90c4922fa8d0f619d2d41 > core/src/test/java/org/apache/oozie/service/TestHadoopAccessorService.java > 96faa4878111baafb81fdf0f3b38becba0c6f15e > core/src/test/java/org/apache/oozie/test/XTestCase.java > e3603692b2ae2ebb741adfa3a6ab5a37ea84eb96 > core/src/test/java/org/apache/oozie/util/TestClasspathUtils.java > PRE-CREATION > sharelib/distcp/pom.xml c8cc47cddc6d2922766e2d68ddddee96f5f3cfec > sharelib/hcatalog/pom.xml 2b0c5042a6f295101f8c5fae0b08a4239b5e97d3 > sharelib/hive/pom.xml d10d7b85b70cbeb1906c83484739f34c09038134 > sharelib/hive2/pom.xml ce967c5575eb639335332dc6de7ce3f3c66a55ca > sharelib/oozie/pom.xml dd95b4537c00b516b124341bdacf4fbd1fd351f9 > > sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/CallbackNotifier.java > PRE-CREATION > sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherAM.java > PRE-CREATION > > sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherMapper.java > f2cba13e89e8ddc16b59a04947309147ea6b34b7 > > sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/PrepareActionsDriver.java > 21ae456b768fb1cb5334a9e23e6c6082240f8dff > sharelib/pig/pom.xml 562c5309cde72c7430fc03f4d342b6d30b5b4575 > sharelib/spark/pom.xml 46c6375314ac314fc6e702695303bbc8eda4f87b > sharelib/sqoop/pom.xml d875c93ae599b990cdcbefdf2aa306eae41a9f0c > sharelib/streaming/pom.xml fd795182f11f0f884177a56592e99aaea01d195d > > Diff: https://reviews.apache.org/r/49778/diff/ > > > Testing > ------- > > - Tried a bunch of stuff against a Hadoop 2.4.0 cluster, including an > "action" that would have different kinds of Exceptions, System.exit, etc. > - Some new and old unit tests. > > > Thanks, > > Robert Kanter > >
