> On 2011-06-12 02:31:03, Todd Lipcon wrote: > > src/java/org/apache/hadoop/mapreduce/JobSubmitter.java, line 400 > > <https://reviews.apache.org/r/885/diff/1/?file=21001#file21001line400> > > > > nit: the javadoc style guide says this should be: > > @param job the job to check for > > > > (so it formats nicely)
Done. > On 2011-06-12 02:31:03, Todd Lipcon wrote: > > src/java/org/apache/hadoop/mapreduce/JobSubmitter.java, lines 401-403 > > <https://reviews.apache.org/r/885/diff/1/?file=21001#file21001line401> > > > > I'm generally against putting empty @throws in JavaDoc - it doesn't > > tell you anything the method signature doesn't already say. I'd just delete > > these lines Agreed. It came on by default in Eclipse, and I had let them be instead of filling it up. Removed. > On 2011-06-12 02:31:03, Todd Lipcon wrote: > > src/java/org/apache/hadoop/mapreduce/JobSubmitter.java, lines 417-420 > > <https://reviews.apache.org/r/885/diff/1/?file=21001#file21001line417> > > > > same comments apply Removed. > On 2011-06-12 02:31:03, Todd Lipcon wrote: > > src/java/org/apache/hadoop/mapreduce/JobSubmitter.java, lines 439-440 > > <https://reviews.apache.org/r/885/diff/1/?file=21001#file21001line439> > > > > rather than this "note" why not just change the above line to "Checks > > if the map-output key and value classes have serializers and deserializers > > associated with them."? Fixed to a more direct message. Sometimes my commenting language is pretty verbose and confusing, thanks for correcting! > On 2011-06-12 02:31:03, Todd Lipcon wrote: > > src/java/org/apache/hadoop/mapreduce/JobSubmitter.java, line 470 > > <https://reviews.apache.org/r/885/diff/1/?file=21001#file21001line470> > > > > missing a space in this exception method. Added. > On 2011-06-12 02:31:03, Todd Lipcon wrote: > > src/test/mapred/org/apache/hadoop/mapreduce/TestMRJobClient.java, line 87 > > <https://reviews.apache.org/r/885/diff/1/?file=21002#file21002line87> > > > > add an assert that e.getMessage().contains("Couldn't find a > > serializer") perhaps? Added as well. - Harsh ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/885/#review806 ----------------------------------------------------------- On 2011-06-11 22:16:24, Harsh Chouraria wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/885/ > ----------------------------------------------------------- > > (Updated 2011-06-11 22:16:24) > > > Review request for hadoop-mapreduce. > > > Summary > ------- > > As discussed on HADOOP-7328, MapReduce can handle serializers in a much > better way in case of bad configuration, improper imports (Some odd Text > class instead of the Writable Text set as key), etc.. > > This issue covers the MapReduce parts of the improvements (made to > MapOutputBuffer and possible early-check of serializer availability > pre-submit) that provide more information than just an NPE as is the current > case. > > > This addresses bug MAPREDUCE-2584. > http://issues.apache.org/jira/browse/MAPREDUCE-2584 > > > Diffs > ----- > > src/java/org/apache/hadoop/mapred/MapTask.java 21599c2 > src/java/org/apache/hadoop/mapreduce/JobSubmitter.java 751d528 > src/test/mapred/org/apache/hadoop/mapreduce/TestMRJobClient.java 5fa329a > > Diff: https://reviews.apache.org/r/885/diff > > > Testing > ------- > > Added a test case that expects a failure if no io.serializers are present. > > > Thanks, > > Harsh > >
