> On 2012-03-08 23:42:23, Dmitriy Lyubimov wrote:
> > trunk/core/src/main/java/org/apache/mahout/common/HadoopUtil.java, line 247
> > <https://reviews.apache.org/r/4237/diff/1/?file=88913#file88913line247>
> >
> >     is this really an issue with Hadoop 0.20.203? i don't think we care 
> > about classic hadoop 0.20.2 anymore.
> 
> tom pierce wrote:
>     Hmm, I am not sure, but I think this is a comment we can omit.  Without 
> the new listStatus methods (but with the changes to other files), there is at 
> least one test that won't compile.  I'm willing to figure out who added this 
> to the patch and ping them about it, but before I do, what should I ask them 
> to clarify?  Exactly which version of Hadoop they had in mind when making the 
> changes to HadoopUtil?

We've misinterpreted the comment - an empty array is returned to emulate 0.20.x 
behavior under 0.23.1.  I applied the patch, reverted HadoopUtil and 
TestDistributedRowMatrix (which is the only class using these new methods), and 
that test still passes under 0.20.x but not under 0.23.1.  From the stacktrace 
it seems fs.listStatus will now throws an exception rather than returning an 
empty array when there are no matches. 

I can see this comment causing the same confusion in the future; I will plan to 
remove the "retain compatibility" comment from HadoopUtil (it appears 2x) 
before committing.


- tom


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


On 2012-03-08 01:24:21, tom pierce wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4237/
> -----------------------------------------------------------
> 
> (Updated 2012-03-08 01:24:21)
> 
> 
> Review request for mahout and Dmitriy Lyubimov.
> 
> 
> Summary
> -------
> 
> This is the current patch for MAHOUT-822 (as posted by Bilung Lee).
> 
> 
> This addresses bug MAHOUT-822.
>     https://issues.apache.org/jira/browse/MAHOUT-822
> 
> 
> Diffs
> -----
> 
>   trunk/core/pom.xml 1296318 
>   trunk/core/src/main/java/org/apache/mahout/common/HadoopUtil.java 1296318 
>   
> trunk/core/src/test/java/org/apache/mahout/classifier/df/mapreduce/partial/MockContext.java
>  1296318 
>   
> trunk/core/src/test/java/org/apache/mahout/classifier/df/mapreduce/partial/PartialSequentialBuilder.java
>  1296318 
>   
> trunk/core/src/test/java/org/apache/mahout/classifier/df/mapreduce/partial/Step1MapperTest.java
>  1296318 
>   
> trunk/core/src/test/java/org/apache/mahout/clustering/canopy/TestCanopyCreation.java
>  1296318 
>   
> trunk/core/src/test/java/org/apache/mahout/clustering/classify/ClusterClassificationDriverTest.java
>  1296318 
>   
> trunk/core/src/test/java/org/apache/mahout/clustering/kmeans/TestKmeansClustering.java
>  1296318 
>   
> trunk/core/src/test/java/org/apache/mahout/clustering/meanshift/TestMeanShift.java
>  1296318 
>   trunk/core/src/test/java/org/apache/mahout/common/DummyCounter.java 1296318 
>   trunk/core/src/test/java/org/apache/mahout/common/DummyRecordWriter.java 
> 1296318 
>   trunk/core/src/test/java/org/apache/mahout/common/DummyStatusReporter.java 
> 1296318 
>   
> trunk/core/src/test/java/org/apache/mahout/math/hadoop/TestDistributedRowMatrix.java
>  1296318 
>   
> trunk/integration/src/test/java/org/apache/mahout/clustering/TestClusterDumper.java
>  1296318 
>   trunk/pom.xml 1296318 
> 
> Diff: https://reviews.apache.org/r/4237/diff
> 
> 
> Testing
> -------
> 
> Passes unit tests under default config as well as under hadoop 0.23.1.
> 
> 
> Thanks,
> 
> tom
> 
>

Reply via email to