----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/10126/#review18414 -----------------------------------------------------------
I love where this is going, awesome work! The only thing that bugs me a bit is all the wrappers - it makes the code a bit hard to understand so I worry about new users. Could we get rid of some of the wrappers if we carry the Iterator<> interface up through to Edge/VertexReader themselves. giraph-hive/src/main/java/org/apache/giraph/hive/common/DefaultConfigurableAndTableSchemaAware.java <https://reviews.apache.org/r/10126/#comment38618> Hehe nice, I was thinking of adding this myself before :) giraph-hive/src/main/java/org/apache/giraph/hive/input/RecordReaderWrapper.java <https://reviews.apache.org/r/10126/#comment38619> Look into Guava's AbstractIterator, it might make this easier/cleaner to implement? giraph-hive/src/main/java/org/apache/giraph/hive/input/edge/AbstractHiveToEdge.java <https://reviews.apache.org/r/10126/#comment38621> Maybe add default implementation of remove() that throws UnsupportedOperationException so others don't have to have the method unless they really want to. Could also make it final if we don't want to allow removal ever? giraph-hive/src/main/java/org/apache/giraph/hive/input/edge/HiveToEdge.java <https://reviews.apache.org/r/10126/#comment38620> Sweet! giraph-hive/src/main/java/org/apache/giraph/hive/input/vertex/HiveToVertex.java <https://reviews.apache.org/r/10126/#comment38622> I take it you removed HiveToVertexValue because you're going to essentially reimplement that with a HiveToEdge that reads single row => multiple edges? Cool! giraph-hive/src/main/java/org/apache/giraph/hive/input/vertex/SimpleNoEdgesHiveToVertex.java <https://reviews.apache.org/r/10126/#comment38625> Prefer ImmutableList.of() - Nitay Joffe On March 25, 2013, 9:53 p.m., Maja Kabiljo wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/10126/ > ----------------------------------------------------------- > > (Updated March 25, 2013, 9:53 p.m.) > > > Review request for giraph and Nitay Joffe. > > > Description > ------- > > Currently with Hive input formats it's only possible to convert single row to > single vertex/edge. We should support some rows to be skipped. General > multiple rows to multiple vertices/edges conversion can be useful (one > example - we have edge input, sorted by source id, and we want to keep only > top k weighted edges per vertex). > > Added some wrappers for readers to convert them to Iterator-like classes. > > > This addresses bug GIRAPH-588. > https://issues.apache.org/jira/browse/GIRAPH-588 > > > Diffs > ----- > > > giraph-core/src/main/java/org/apache/giraph/io/iterables/EdgeReaderWrapper.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/io/iterables/EdgeWithSource.java > PRE-CREATION > giraph-core/src/main/java/org/apache/giraph/io/iterables/GiraphReader.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/io/iterables/IteratorToReaderWrapper.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/io/iterables/VertexReaderWrapper.java > PRE-CREATION > giraph-core/src/main/java/org/apache/giraph/io/iterables/package-info.java > PRE-CREATION > giraph-hive/src/main/java/org/apache/giraph/hive/HiveGiraphRunner.java > efc08d3 > > giraph-hive/src/main/java/org/apache/giraph/hive/common/DefaultConfigurableAndTableSchemaAware.java > PRE-CREATION > > giraph-hive/src/main/java/org/apache/giraph/hive/input/RecordReaderWrapper.java > PRE-CREATION > > giraph-hive/src/main/java/org/apache/giraph/hive/input/edge/AbstractHiveToEdge.java > f29fea7 > > giraph-hive/src/main/java/org/apache/giraph/hive/input/edge/HiveEdgeInputFormat.java > 18b40c2 > > giraph-hive/src/main/java/org/apache/giraph/hive/input/edge/HiveEdgeReader.java > 6fb183a > giraph-hive/src/main/java/org/apache/giraph/hive/input/edge/HiveToEdge.java > 2205b82 > > giraph-hive/src/main/java/org/apache/giraph/hive/input/edge/SimpleHiveToEdge.java > PRE-CREATION > > giraph-hive/src/main/java/org/apache/giraph/hive/input/vertex/AbstractHiveToVertex.java > PRE-CREATION > > giraph-hive/src/main/java/org/apache/giraph/hive/input/vertex/AbstractHiveToVertexEdges.java > d0668f6 > > giraph-hive/src/main/java/org/apache/giraph/hive/input/vertex/AbstractHiveToVertexValue.java > 9ab316f > > giraph-hive/src/main/java/org/apache/giraph/hive/input/vertex/HiveToVertex.java > PRE-CREATION > > giraph-hive/src/main/java/org/apache/giraph/hive/input/vertex/HiveToVertexEdges.java > 8076a8a > > giraph-hive/src/main/java/org/apache/giraph/hive/input/vertex/HiveToVertexValue.java > 593eb9a > > giraph-hive/src/main/java/org/apache/giraph/hive/input/vertex/HiveVertexInputFormat.java > 25c7a26 > > giraph-hive/src/main/java/org/apache/giraph/hive/input/vertex/HiveVertexReader.java > 541176f > > giraph-hive/src/main/java/org/apache/giraph/hive/input/vertex/SimpleHiveToVertex.java > PRE-CREATION > > giraph-hive/src/main/java/org/apache/giraph/hive/input/vertex/SimpleNoEdgesHiveToVertex.java > PRE-CREATION > > giraph-hive/src/main/java/org/apache/giraph/hive/output/AbstractVertexToHive.java > 8e3f1ca > > Diff: https://reviews.apache.org/r/10126/diff/ > > > Testing > ------- > > > Thanks, > > Maja Kabiljo > >
