----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/9350/ -----------------------------------------------------------
(Updated Feb. 8, 2013, 9:48 p.m.) Review request for giraph. Changes ------- Sorry, realized there was one more thing to do to really finish this. Now the CliParserUtils are called ConfigurationUtils, because they encapsulate parsing of the command line (including accepting pre-parse custom options and returning a CommandLine object that can be further queried by custom Runner/Job/whatever code) but also composing the GiraphConfiguration for our job using this parser, and validating all global config settings that will be needed for any platform Giraph runs on (YARN, Hadoop, Mesos, whatever.) This makes future refactors of Runner or Job (either to consolidate or to revamp existing impls like HiveGiraphRunner) easy since Runner and Job using these utils are now very short and clear, pretty much only containing code that meets their basic interface and/or is custom for the platform (Hadoop MR in the case of GiraphJob/GiraphRunner.) This passes 'mvn verify' as well as real cluster tests. I promise I am done "fixing" it other than what you want done under review! Description ------- Refactor CLI parsing of args and the assembly of the GiraphConfiguration into its own class so that other future Runner classes besides GiraphRunner can use it w/o duplication. You get the idea. Diffs (updated) ----- giraph-core/src/main/java/org/apache/giraph/GiraphRunner.java b6a6113 giraph-core/src/main/java/org/apache/giraph/job/GiraphConfigurationValidator.java PRE-CREATION giraph-core/src/main/java/org/apache/giraph/job/GiraphJob.java 9f711da giraph-core/src/main/java/org/apache/giraph/job/GiraphTypeValidator.java de2a66d giraph-core/src/main/java/org/apache/giraph/utils/ConfigurationUtils.java PRE-CREATION giraph-core/src/test/java/org/apache/giraph/vertex/TestVertexTypes.java 80187ef Diff: https://reviews.apache.org/r/9350/diff/ Testing ------- Thanks, Eli Reisman
