[jira] [Commented] (GIRAPH-144) GiraphJob should not extend Job (users should not be able to call Job methods like waitForCompletion or setMapper..etc)

2012-03-27 Thread Hudson (Commented) (JIRA)

[ 
https://issues.apache.org/jira/browse/GIRAPH-144?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13239945#comment-13239945
 ] 

Hudson commented on GIRAPH-144:
---

Integrated in Giraph-trunk-Commit #95 (See 
[https://builds.apache.org/job/Giraph-trunk-Commit/95/])
GIRAPH-144: GiraphJob should not extend Job (users should not be able
to call Job methods like waitForCompletion or setMapper..etc)
(aching). (Revision 1306014)

 Result = SUCCESS
aching : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1306014
Files : 
* /incubator/giraph/trunk/CHANGELOG
* /incubator/giraph/trunk/src/main/java/org/apache/giraph/GiraphRunner.java
* 
/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleCheckpointVertex.java
* 
/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleShortestPathsVertex.java
* /incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/GiraphJob.java
* 
/incubator/giraph/trunk/src/main/java/org/apache/giraph/utils/InternalVertexRunner.java
* /incubator/giraph/trunk/src/test/java/org/apache/giraph/BspCase.java
* 
/incubator/giraph/trunk/src/test/java/org/apache/giraph/TestJsonBase64Format.java


> GiraphJob should not extend Job  (users should not be able to call Job 
> methods like waitForCompletion or setMapper..etc)
> 
>
> Key: GIRAPH-144
> URL: https://issues.apache.org/jira/browse/GIRAPH-144
> Project: Giraph
>  Issue Type: Bug
>Reporter: Dave
>Assignee: Avery Ching
> Attachments: GIRAPH-144.patch
>
>   Original Estimate: 24h
>  Remaining Estimate: 24h
>


--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira




[jira] [Commented] (GIRAPH-144) GiraphJob should not extend Job (users should not be able to call Job methods like waitForCompletion or setMapper..etc)

2012-03-25 Thread Jakob Homan (Commented) (JIRA)

[ 
https://issues.apache.org/jira/browse/GIRAPH-144?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13237969#comment-13237969
 ] 

Jakob Homan commented on GIRAPH-144:


Sorry, had missed the first ping.  Composition is definitely the way to go, I'm 
just still a bit worried about exposing the underlying Job.  Once we go to 
YARN, there's no reason to think that there will be a Job anymore to expose.  
It might be time to pull in Hadoop's interface classifications and mark that 
method as private unstable...  But this approach is better than what we have 
now, so go ahead with it until we come up with something better.

> GiraphJob should not extend Job  (users should not be able to call Job 
> methods like waitForCompletion or setMapper..etc)
> 
>
> Key: GIRAPH-144
> URL: https://issues.apache.org/jira/browse/GIRAPH-144
> Project: Giraph
>  Issue Type: Bug
>Reporter: Dave
>Assignee: Avery Ching
> Attachments: GIRAPH-144.patch
>
>   Original Estimate: 24h
>  Remaining Estimate: 24h
>


--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira




[jira] [Commented] (GIRAPH-144) GiraphJob should not extend Job (users should not be able to call Job methods like waitForCompletion or setMapper..etc)

2012-03-24 Thread Avery Ching (Commented) (JIRA)

[ 
https://issues.apache.org/jira/browse/GIRAPH-144?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13237597#comment-13237597
 ] 

Avery Ching commented on GIRAPH-144:


Ping, anyone?  I'd like to close this out, one way or another.

> GiraphJob should not extend Job  (users should not be able to call Job 
> methods like waitForCompletion or setMapper..etc)
> 
>
> Key: GIRAPH-144
> URL: https://issues.apache.org/jira/browse/GIRAPH-144
> Project: Giraph
>  Issue Type: Bug
>Reporter: Dave
>Assignee: Avery Ching
> Attachments: GIRAPH-144.patch
>
>   Original Estimate: 24h
>  Remaining Estimate: 24h
>


--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira




[jira] [Commented] (GIRAPH-144) GiraphJob should not extend Job (users should not be able to call Job methods like waitForCompletion or setMapper..etc)

2012-03-07 Thread Avery Ching (Commented) (JIRA)

[ 
https://issues.apache.org/jira/browse/GIRAPH-144?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13224096#comment-13224096
 ] 

Avery Ching commented on GIRAPH-144:


@Jakob, any more thoughts?

> GiraphJob should not extend Job  (users should not be able to call Job 
> methods like waitForCompletion or setMapper..etc)
> 
>
> Key: GIRAPH-144
> URL: https://issues.apache.org/jira/browse/GIRAPH-144
> Project: Giraph
>  Issue Type: Bug
>Reporter: Dave
>Assignee: Avery Ching
> Attachments: GIRAPH-144.patch
>
>   Original Estimate: 24h
>  Remaining Estimate: 24h
>


--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira




[jira] [Commented] (GIRAPH-144) GiraphJob should not extend Job (users should not be able to call Job methods like waitForCompletion or setMapper..etc)

2012-02-09 Thread Avery Ching (Commented) (JIRA)

[ 
https://issues.apache.org/jira/browse/GIRAPH-144?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13205022#comment-13205022
 ] 

Avery Ching commented on GIRAPH-144:


There are basically two approaches we can have here to prevent users from doing 
the wrong thing with GiraphJob: composition or inheritance (extends)

If we use composition, the benefits are that we only expose methods we want and 
users aren't exposed to Job methods directly that they shouldn't be.  The 
downside is that the Job is required sometimes (i.e. 
FileInputFormat#addInputPaths(Job job...)).  In this case, you can access the 
internal Job with GiraphJob#getInternalJob(), but the method is well documented 
that you should try to avoid using this directly.

If we choose inheritance (current approach), then we get to use 
FileInputFormat#addInputPath on GiraphJob directly, but should override every 
method by Job (and its parents) and prevent clients from using it by throwing 
exceptions.  This is a big ugly and depending on the version of Hadoop, is 
likely to have slightly different methods.

In my opinion, I prefer composition (see Joshua Bloch's Effective Java : Item 
16).  Issues will be found at compile time  for composition (i.e. 
GiraphJob#submit() doesn't exist and will be a compile bug) rather than at 
runtime for inheritance (GiraphJob#submit() throws an Exception).  Again, if 
users really need the Job, we do expose it in GiraphJob via the getter method 
and the javadoc should provide a warning to be careful.



> GiraphJob should not extend Job  (users should not be able to call Job 
> methods like waitForCompletion or setMapper..etc)
> 
>
> Key: GIRAPH-144
> URL: https://issues.apache.org/jira/browse/GIRAPH-144
> Project: Giraph
>  Issue Type: Bug
>Reporter: Dave
>Assignee: Avery Ching
> Attachments: GIRAPH-144.patch
>
>   Original Estimate: 24h
>  Remaining Estimate: 24h
>


--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira




[jira] [Commented] (GIRAPH-144) GiraphJob should not extend Job (users should not be able to call Job methods like waitForCompletion or setMapper..etc)

2012-02-09 Thread Jakob Homan (Commented) (JIRA)

[ 
https://issues.apache.org/jira/browse/GIRAPH-144?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13204818#comment-13204818
 ] 

Jakob Homan commented on GIRAPH-144:


bq. I have already submitted a patch that handles this with composition, but 
exposes the internal job object when necessary (i.e. 
FileInputFormat.addInputPath).  This should give us the benefits of composition 
while allowing us to still call convenience methods on the Job object.
Yeah... hmmm... Exposing the field via a getter method bugs me a bit.  It seems 
like the worst of both worlds; GiraphJob is no longer a Job, and we don't have 
any control over the Job that is being shown.  

> GiraphJob should not extend Job  (users should not be able to call Job 
> methods like waitForCompletion or setMapper..etc)
> 
>
> Key: GIRAPH-144
> URL: https://issues.apache.org/jira/browse/GIRAPH-144
> Project: Giraph
>  Issue Type: Bug
>Reporter: Dave
>Assignee: Avery Ching
> Attachments: GIRAPH-144.patch
>
>   Original Estimate: 24h
>  Remaining Estimate: 24h
>


--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira




[jira] [Commented] (GIRAPH-144) GiraphJob should not extend Job (users should not be able to call Job methods like waitForCompletion or setMapper..etc)

2012-02-09 Thread jirapos...@reviews.apache.org (Commented) (JIRA)

[ 
https://issues.apache.org/jira/browse/GIRAPH-144?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13204378#comment-13204378
 ] 

jirapos...@reviews.apache.org commented on GIRAPH-144:
--


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

Review request for giraph.


Summary
---

GiraphJob uses composition with Job instead of extending Job (exposing all Job 
methods to users is bad).


This addresses bug GIRAPH-144.
https://issues.apache.org/jira/browse/GIRAPH-144


Diffs
-

  
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/utils/InternalVertexRunner.java
 1241611 
  
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/BspCase.java
 1241611 
  
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/test/java/org/apache/giraph/TestJsonBase64Format.java
 1241611 
  
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleVertexWithWorkerContext.java
 1241611 
  
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/graph/GiraphJob.java
 1241611 
  
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/GiraphRunner.java
 1241611 
  
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleCheckpointVertex.java
 1241611 
  
http://svn.apache.org/repos/asf/incubator/giraph/trunk/src/main/java/org/apache/giraph/examples/SimpleShortestPathsVertex.java
 1241611 

Diff: https://reviews.apache.org/r/3823/diff


Testing
---

local and MR unittests.


Thanks,

Avery



> GiraphJob should not extend Job  (users should not be able to call Job 
> methods like waitForCompletion or setMapper..etc)
> 
>
> Key: GIRAPH-144
> URL: https://issues.apache.org/jira/browse/GIRAPH-144
> Project: Giraph
>  Issue Type: Bug
>Reporter: Dave
>Assignee: Avery Ching
> Attachments: GIRAPH-144.patch
>
>   Original Estimate: 24h
>  Remaining Estimate: 24h
>


--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira




[jira] [Commented] (GIRAPH-144) GiraphJob should not extend Job (users should not be able to call Job methods like waitForCompletion or setMapper..etc)

2012-02-08 Thread Avery Ching (Commented) (JIRA)

[ 
https://issues.apache.org/jira/browse/GIRAPH-144?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13204213#comment-13204213
 ] 

Avery Ching commented on GIRAPH-144:


I'm working on this, should have a fix by tonight.

> GiraphJob should not extend Job  (users should not be able to call Job 
> methods like waitForCompletion or setMapper..etc)
> 
>
> Key: GIRAPH-144
> URL: https://issues.apache.org/jira/browse/GIRAPH-144
> Project: Giraph
>  Issue Type: Bug
>Reporter: Dave
>Assignee: Avery Ching
>   Original Estimate: 24h
>  Remaining Estimate: 24h
>


--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira