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 Tue, 12 Feb 2013 18:14:14 GMT


> On Feb. 7, 2013, 4:14 a.m., Nitay Joffe wrote:
> > giraph-core/src/main/java/org/apache/giraph/GiraphRunner.java, line 159
> > <https://reviews.apache.org/r/9350/diff/1/?file=256469#file256469line159>
> >
> >     We have another runner for Hive jobs, called HiveGiraphRunner. You should take
a look at that as well. At Facebook we also have our own Runner that inherits from HiveGiraphRunner.
To be honest both of these classes (ours and HiveGiraphRunner) need to go away in my opinion.
Instead we should be able to just plugin additional options parsers, so that if you want Hive
you plugin some HiveOptionsParser, we'll have our CompanyOptionsParser... but everyone will
use the same one modular GiraphRunner. Does that make sense? Anyways this can be in a separate
issue but wanted to get you thinking about it.

yes (see other comment below) I think this is what we should move towards. I think in its
most recent form, this is moving in that direction. 


- Eli


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


On Feb. 8, 2013, 9:48 p.m., Eli Reisman wrote:
> 
> -----------------------------------------------------------
> 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.
> 
> 
> 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
> -----
> 
>   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