> On Aug. 18, 2013, 8:58 p.m., Rohini Palaniswamy wrote: > > trunk/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java, > > lines 1030-1034 > > <https://reviews.apache.org/r/13397/diff/3/?file=341729#file341729line1030> > > > > Don't have to make hasIdSwap call and check the counter. You can just > > check if the map has the key. Please create another jira to remove code > > related to setting and checking of the counter or if possible do it in this > > jira itself.
> On Aug. 18, 2013, 8:58 p.m., Rohini Palaniswamy wrote: > > trunk/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java, > > line 1066 > > <https://reviews.apache.org/r/13397/diff/3/?file=341729#file341729line1066> > > > > If there was an id swap, i think you need to reload the action data > > here. hmm think so too. > On Aug. 18, 2013, 8:58 p.m., Rohini Palaniswamy wrote: > > trunk/core/src/main/java/org/apache/oozie/action/hadoop/LauncherMapperHelper.java, > > line 277 > > <https://reviews.apache.org/r/13397/diff/3/?file=341730#file341730line277> > > > > Do a file exists check and return empty map if the file does not exist. > > The file will not exist if the launcher did not run at all. > > > > There should be no fs.getHomeDirectory(). The path should just be > > getActionDataSequenceFilePath(actionDir) ah. maybe thats why i was getting filenotfound error > On Aug. 18, 2013, 8:58 p.m., Rohini Palaniswamy wrote: > > trunk/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherMapper.java, > > line 85 > > <https://reviews.apache.org/r/13397/diff/3/?file=341735#file341735line85> > > > > The file name will become oozie.action.data.seq. Just have > > action-data.seq or action_data.seq. No . in the middle of the file name. this is what happens when one burns the midnight oil :/ > On Aug. 18, 2013, 8:58 p.m., Rohini Palaniswamy wrote: > > trunk/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherMapper.java, > > lines 347-357 > > <https://reviews.apache.org/r/13397/diff/3/?file=341735#file341735line347> > > > > This should be in a finally block after LauncherException catch block. > > You are only creating the file in case of an exception now. same as above. > On Aug. 18, 2013, 8:58 p.m., Rohini Palaniswamy wrote: > > trunk/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherMapper.java, > > line 435 > > <https://reviews.apache.org/r/13397/diff/3/?file=341735#file341735line435> > > > > You don't need hadoop filesystem to read a File. Just read using > > BufferedInputStream and FileInputStream. Also do not read line by line. > > Read as is into byte array. Currently you are reading line by line and > > appending to stringbuffer without appending \n. > > > > Can do FileUtils.readFileToString() from commons-io package, but > > commons-io is not present in hadoop 1.x. Only in 0.23. okay thanks - Mona ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/13397/#review25280 ----------------------------------------------------------- On Aug. 16, 2013, 9:31 a.m., Mona Chitnis wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/13397/ > ----------------------------------------------------------- > > (Updated Aug. 16, 2013, 9:31 a.m.) > > > Review request for oozie. > > > Bugs: OOZIE-1486 > https://issues.apache.org/jira/browse/OOZIE-1486 > > > Repository: oozie > > > Description > ------- > > See JIRA Description > > > Diffs > ----- > > > trunk/core/src/main/java/org/apache/oozie/action/hadoop/HiveActionExecutor.java > 1514596 > > trunk/core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java > 1514596 > > trunk/core/src/main/java/org/apache/oozie/action/hadoop/LauncherMapperHelper.java > 1514596 > > trunk/core/src/main/java/org/apache/oozie/action/hadoop/PigActionExecutor.java > 1514596 > > trunk/core/src/main/java/org/apache/oozie/action/hadoop/SqoopActionExecutor.java > 1514596 > trunk/core/src/test/java/org/apache/oozie/action/hadoop/TestLauncher.java > 1514596 > > trunk/sharelib/hive/src/main/java/org/apache/oozie/action/hadoop/HiveMain.java > 1514596 > > trunk/sharelib/oozie/src/main/java/org/apache/oozie/action/hadoop/LauncherMapper.java > 1514596 > > Diff: https://reviews.apache.org/r/13397/diff/ > > > Testing > ------- > > Existing unit tests should pass, and E-2-E test pending. > > Few tests failing with a common problem of Properties object (for > output/new-id/error etc) getting stored in the file with Timestamp. Hence on > retrieval, it gives NPE on the expected keys. WIP to fix that small part > > > Thanks, > > Mona Chitnis > >
