You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@giraph.apache.org by Alessandro Presta <al...@fb.com> on 2013/03/25 17:36:02 UTC
Review Request: VertexValueFactory
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10119/
-----------------------------------------------------------
Review request for giraph.
Description
-------
Adds a VertexValueFactory that can be optionally specified by the user.
It's initialized with the configuration.
DefaultVertexValueFactory implements the current behavior (caches the vertex value class and uses reflection).
This addresses bug GIRAPH-586.
https://issues.apache.org/jira/browse/GIRAPH-586
Diffs
-----
giraph-core/src/main/java/org/apache/giraph/conf/GiraphClasses.java 5090250d93ec73be34d0bd306d00704856a89785
giraph-core/src/main/java/org/apache/giraph/conf/GiraphConfiguration.java 6886d5827b3aeece07bbf192390171dc17a05e1d
giraph-core/src/main/java/org/apache/giraph/conf/GiraphConstants.java ad9073d2431f4e065f373468a54cd2b9baa7c654
giraph-core/src/main/java/org/apache/giraph/conf/ImmutableClassesGiraphConfiguration.java 4fedc464ce08051cdadb49cc0ac2e8553093ce46
giraph-core/src/main/java/org/apache/giraph/graph/DefaultVertexValueFactory.java PRE-CREATION
giraph-core/src/main/java/org/apache/giraph/graph/VertexValueFactory.java PRE-CREATION
giraph-core/src/main/java/org/apache/giraph/job/GiraphConfigurationValidator.java b08c74d42808790f1eb860183feed436b53c5275
giraph-core/src/main/java/org/apache/giraph/utils/ConfigurationUtils.java 41238d045c41c8292a8ea70ba899fa51cfe42480
giraph-core/src/main/java/org/apache/giraph/utils/InternalVertexRunner.java 7e0b955998a5f1890912819fa2ca74fbbf46fedd
giraph-core/src/test/java/org/apache/giraph/io/TestEdgeInput.java 55c0c86ee7f188d6624def8dd3002962631d6a3f
giraph-examples/src/test/java/org/apache/giraph/vertex/TestVertexTypes.java c47c291e3102619319667ace594c18b7c6cb442b
Diff: https://reviews.apache.org/r/10119/diff/
Testing
-------
mvn verify
Thanks,
Alessandro Presta
Re: Review Request: VertexValueFactory
Posted by Nitay Joffe <ni...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10119/#review18354
-----------------------------------------------------------
giraph-core/src/main/java/org/apache/giraph/graph/VertexValueFactory.java
<https://reviews.apache.org/r/10119/#comment38556>
why not an interface?
- Nitay Joffe
On March 25, 2013, 4:36 p.m., Alessandro Presta wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10119/
> -----------------------------------------------------------
>
> (Updated March 25, 2013, 4:36 p.m.)
>
>
> Review request for giraph.
>
>
> Description
> -------
>
> Adds a VertexValueFactory that can be optionally specified by the user.
> It's initialized with the configuration.
> DefaultVertexValueFactory implements the current behavior (caches the vertex value class and uses reflection).
>
>
> This addresses bug GIRAPH-586.
> https://issues.apache.org/jira/browse/GIRAPH-586
>
>
> Diffs
> -----
>
> giraph-core/src/main/java/org/apache/giraph/conf/GiraphClasses.java 5090250d93ec73be34d0bd306d00704856a89785
> giraph-core/src/main/java/org/apache/giraph/conf/GiraphConfiguration.java 6886d5827b3aeece07bbf192390171dc17a05e1d
> giraph-core/src/main/java/org/apache/giraph/conf/GiraphConstants.java ad9073d2431f4e065f373468a54cd2b9baa7c654
> giraph-core/src/main/java/org/apache/giraph/conf/ImmutableClassesGiraphConfiguration.java 4fedc464ce08051cdadb49cc0ac2e8553093ce46
> giraph-core/src/main/java/org/apache/giraph/graph/DefaultVertexValueFactory.java PRE-CREATION
> giraph-core/src/main/java/org/apache/giraph/graph/VertexValueFactory.java PRE-CREATION
> giraph-core/src/main/java/org/apache/giraph/job/GiraphConfigurationValidator.java b08c74d42808790f1eb860183feed436b53c5275
> giraph-core/src/main/java/org/apache/giraph/utils/ConfigurationUtils.java 41238d045c41c8292a8ea70ba899fa51cfe42480
> giraph-core/src/main/java/org/apache/giraph/utils/InternalVertexRunner.java 7e0b955998a5f1890912819fa2ca74fbbf46fedd
> giraph-core/src/test/java/org/apache/giraph/io/TestEdgeInput.java 55c0c86ee7f188d6624def8dd3002962631d6a3f
> giraph-examples/src/test/java/org/apache/giraph/vertex/TestVertexTypes.java c47c291e3102619319667ace594c18b7c6cb442b
>
> Diff: https://reviews.apache.org/r/10119/diff/
>
>
> Testing
> -------
>
> mvn verify
>
>
> Thanks,
>
> Alessandro Presta
>
>
Re: Review Request: VertexValueFactory
Posted by Alessandro Presta <al...@fb.com>.
> On March 25, 2013, 4:57 p.m., Avery Ching wrote:
> > giraph-core/src/main/java/org/apache/giraph/conf/ImmutableClassesGiraphConfiguration.java, line 97
> > <https://reviews.apache.org/r/10119/diff/1/?file=274344#file274344line97>
> >
> > Can you please change this exception message to something like "ImmutableClassesGiraphConfiguration: Failed to instantiate class " + classes.getVertexValueFactoryClass()
The dangers of copy-paste... Thanks!
- Alessandro
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10119/#review18341
-----------------------------------------------------------
On March 25, 2013, 4:36 p.m., Alessandro Presta wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10119/
> -----------------------------------------------------------
>
> (Updated March 25, 2013, 4:36 p.m.)
>
>
> Review request for giraph.
>
>
> Description
> -------
>
> Adds a VertexValueFactory that can be optionally specified by the user.
> It's initialized with the configuration.
> DefaultVertexValueFactory implements the current behavior (caches the vertex value class and uses reflection).
>
>
> This addresses bug GIRAPH-586.
> https://issues.apache.org/jira/browse/GIRAPH-586
>
>
> Diffs
> -----
>
> giraph-core/src/main/java/org/apache/giraph/conf/GiraphClasses.java 5090250d93ec73be34d0bd306d00704856a89785
> giraph-core/src/main/java/org/apache/giraph/conf/GiraphConfiguration.java 6886d5827b3aeece07bbf192390171dc17a05e1d
> giraph-core/src/main/java/org/apache/giraph/conf/GiraphConstants.java ad9073d2431f4e065f373468a54cd2b9baa7c654
> giraph-core/src/main/java/org/apache/giraph/conf/ImmutableClassesGiraphConfiguration.java 4fedc464ce08051cdadb49cc0ac2e8553093ce46
> giraph-core/src/main/java/org/apache/giraph/graph/DefaultVertexValueFactory.java PRE-CREATION
> giraph-core/src/main/java/org/apache/giraph/graph/VertexValueFactory.java PRE-CREATION
> giraph-core/src/main/java/org/apache/giraph/job/GiraphConfigurationValidator.java b08c74d42808790f1eb860183feed436b53c5275
> giraph-core/src/main/java/org/apache/giraph/utils/ConfigurationUtils.java 41238d045c41c8292a8ea70ba899fa51cfe42480
> giraph-core/src/main/java/org/apache/giraph/utils/InternalVertexRunner.java 7e0b955998a5f1890912819fa2ca74fbbf46fedd
> giraph-core/src/test/java/org/apache/giraph/io/TestEdgeInput.java 55c0c86ee7f188d6624def8dd3002962631d6a3f
> giraph-examples/src/test/java/org/apache/giraph/vertex/TestVertexTypes.java c47c291e3102619319667ace594c18b7c6cb442b
>
> Diff: https://reviews.apache.org/r/10119/diff/
>
>
> Testing
> -------
>
> mvn verify
>
>
> Thanks,
>
> Alessandro Presta
>
>
Re: Review Request: VertexValueFactory
Posted by Avery Ching <av...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10119/#review18341
-----------------------------------------------------------
Ship it!
+1! This looks great. Minor comments are about error messages.
giraph-core/src/main/java/org/apache/giraph/conf/ImmutableClassesGiraphConfiguration.java
<https://reviews.apache.org/r/10119/#comment38554>
Can you please change this exception message to something like "ImmutableClassesGiraphConfiguration: Failed to instantiate class " + classes.getVertexValueFactoryClass()
giraph-core/src/main/java/org/apache/giraph/conf/ImmutableClassesGiraphConfiguration.java
<https://reviews.apache.org/r/10119/#comment38553>
Can you please change this exception message to something like "ImmutableClassesGiraphConfiguration: Illegally accessed class " + classes.getVertexValueFactoryClass()
- Avery Ching
On March 25, 2013, 4:36 p.m., Alessandro Presta wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10119/
> -----------------------------------------------------------
>
> (Updated March 25, 2013, 4:36 p.m.)
>
>
> Review request for giraph.
>
>
> Description
> -------
>
> Adds a VertexValueFactory that can be optionally specified by the user.
> It's initialized with the configuration.
> DefaultVertexValueFactory implements the current behavior (caches the vertex value class and uses reflection).
>
>
> This addresses bug GIRAPH-586.
> https://issues.apache.org/jira/browse/GIRAPH-586
>
>
> Diffs
> -----
>
> giraph-core/src/main/java/org/apache/giraph/conf/GiraphClasses.java 5090250d93ec73be34d0bd306d00704856a89785
> giraph-core/src/main/java/org/apache/giraph/conf/GiraphConfiguration.java 6886d5827b3aeece07bbf192390171dc17a05e1d
> giraph-core/src/main/java/org/apache/giraph/conf/GiraphConstants.java ad9073d2431f4e065f373468a54cd2b9baa7c654
> giraph-core/src/main/java/org/apache/giraph/conf/ImmutableClassesGiraphConfiguration.java 4fedc464ce08051cdadb49cc0ac2e8553093ce46
> giraph-core/src/main/java/org/apache/giraph/graph/DefaultVertexValueFactory.java PRE-CREATION
> giraph-core/src/main/java/org/apache/giraph/graph/VertexValueFactory.java PRE-CREATION
> giraph-core/src/main/java/org/apache/giraph/job/GiraphConfigurationValidator.java b08c74d42808790f1eb860183feed436b53c5275
> giraph-core/src/main/java/org/apache/giraph/utils/ConfigurationUtils.java 41238d045c41c8292a8ea70ba899fa51cfe42480
> giraph-core/src/main/java/org/apache/giraph/utils/InternalVertexRunner.java 7e0b955998a5f1890912819fa2ca74fbbf46fedd
> giraph-core/src/test/java/org/apache/giraph/io/TestEdgeInput.java 55c0c86ee7f188d6624def8dd3002962631d6a3f
> giraph-examples/src/test/java/org/apache/giraph/vertex/TestVertexTypes.java c47c291e3102619319667ace594c18b7c6cb442b
>
> Diff: https://reviews.apache.org/r/10119/diff/
>
>
> Testing
> -------
>
> mvn verify
>
>
> Thanks,
>
> Alessandro Presta
>
>