----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/10564/#review19954 -----------------------------------------------------------
This is looking much cleaner now. giraph-core/src/main/java/org/apache/giraph/io/formats/multi/EdgeInputFormatDescription.java <https://reviews.apache.org/r/10564/#comment41184> I would specify in the javadoc that this is only relevant to multiple inputs (I know it's in "multi", but it would make it clearer). giraph-core/src/main/java/org/apache/giraph/io/formats/multi/EdgeInputFormatDescription.java <https://reviews.apache.org/r/10564/#comment41187> "ret" is not a great name for a variable. How about "edgeInputFormats"? giraph-core/src/main/java/org/apache/giraph/io/formats/multi/InputFormatDescription.java <https://reviews.apache.org/r/10564/#comment41185> Same as for EdgeInputFormatDescription. giraph-hive/src/main/java/org/apache/giraph/hive/HiveGiraphRunner.java <https://reviews.apache.org/r/10564/#comment41188> I think this multitude of comma-separated options, where order is relevant, is not that robust. I'd prefer a representation with tuples (format, table, filter) akin to what you did with the multi-input description in giraph-core. It doesn't have to be one JSON object. You could make it so that an arbitrary number of inputs can be specified. Example: -edgeInput (MyFormat, my_table, my_partition) -edgeInput (MyOtherFormat, my_other_table, my_other_partition) ... - Alessandro Presta On April 30, 2013, 7:05 a.m., Maja Kabiljo wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/10564/ > ----------------------------------------------------------- > > (Updated April 30, 2013, 7:05 a.m.) > > > Review request for giraph. > > > Description > ------- > > For now, I did this only for Edge input, once I get some feedback I'll do the > exactly same thing for vertex input. > Also, I added direct support only to HiveGiraphRunner, we can extend it later > to others as well. > > > This addresses bug GIRAPH-639. > https://issues.apache.org/jira/browse/GIRAPH-639 > > > Diffs > ----- > > giraph-core/src/main/java/org/apache/giraph/conf/GiraphConfiguration.java > 795cd0c > > giraph-core/src/main/java/org/apache/giraph/conf/ImmutableClassesGiraphConfiguration.java > f5a926f > giraph-core/src/main/java/org/apache/giraph/io/EdgeInputFormat.java 2aac1f0 > giraph-core/src/main/java/org/apache/giraph/io/GiraphInputFormat.java > 13efc93 > giraph-core/src/main/java/org/apache/giraph/io/VertexInputFormat.java > c4d7fe2 > > giraph-core/src/main/java/org/apache/giraph/io/formats/multi/EdgeInputFormatDescription.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/io/formats/multi/InputFormatDescription.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/io/formats/multi/MultiEdgeInputFormat.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/io/formats/multi/package-info.java > PRE-CREATION > > giraph-core/src/main/java/org/apache/giraph/io/internal/WrappedEdgeInputFormat.java > 9b14727 > giraph-core/src/main/java/org/apache/giraph/master/BspServiceMaster.java > 2c1a679 > giraph-core/src/main/java/org/apache/giraph/worker/BspServiceWorker.java > 01937ab > > giraph-core/src/main/java/org/apache/giraph/worker/EdgeInputSplitsCallable.java > de1e774 > > giraph-core/src/main/java/org/apache/giraph/worker/EdgeInputSplitsCallableFactory.java > 4a1705b > giraph-core/src/main/java/org/apache/giraph/worker/InputSplitsCallable.java > a3a9ab7 > > giraph-core/src/main/java/org/apache/giraph/worker/VertexInputSplitsCallable.java > 224856e > > giraph-core/src/main/java/org/apache/giraph/worker/VertexInputSplitsCallableFactory.java > 4eff3b8 > giraph-hive/src/main/java/org/apache/giraph/hive/HiveGiraphRunner.java > 8d67c1d > > Diff: https://reviews.apache.org/r/10564/diff/ > > > Testing > ------- > > mvn clean verify > Run application with two edge input tables - verified results. > > > Thanks, > > Maja Kabiljo > >
