giraph-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Eli Reisman" <initialcont...@gmail.com>
Subject Re: Review Request: Refactor CLI arg parsing out of GiraphRunner
Date Fri, 08 Feb 2013 21:48:12 GMT

-----------------------------------------------------------
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


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message