You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@crunch.apache.org by "Rahul Sharma (JIRA)" <ji...@apache.org> on 2012/10/10 19:33:03 UTC
[jira] [Created] (CRUNCH-93) Crunch core source should not contain
packages like o.a.c.test
Rahul Sharma created CRUNCH-93:
----------------------------------
Summary: Crunch core source should not contain packages like o.a.c.test
Key: CRUNCH-93
URL: https://issues.apache.org/jira/browse/CRUNCH-93
Project: Crunch
Issue Type: Bug
Reporter: Rahul Sharma
Priority: Minor
Crunch core has the following things a package named o.a.c.test and TestCounters class.The source should not contain test package and classes named with Test.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (CRUNCH-93) Crunch core source should not contain
packages like o.a.c.test
Posted by "Matthias Friedrich (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CRUNCH-93?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Matthias Friedrich updated CRUNCH-93:
-------------------------------------
Attachment: CRUNCH-97-mafr.patch
+1. Just had a look at it, this is really good work, I like how it simplifies DoFn. I'm willing to pay the price of pulling in another dependency since it's limited to MemCollection and javassist is distributed under the Apache License (among others). After all, we can get rid of it in the distant future when we don't have to support Hadoop 1 anymore.
I've made a few cosmetic changes and reformatted your commit message (there was a newline missing which leads to git putting everything into a single line). Hope you don't mind.
> Crunch core source should not contain packages like o.a.c.test
> ---------------------------------------------------------------
>
> Key: CRUNCH-93
> URL: https://issues.apache.org/jira/browse/CRUNCH-93
> Project: Crunch
> Issue Type: Bug
> Reporter: Rahul Sharma
> Priority: Minor
> Attachments: 0001-CRUNCH-93-moving-test-support-classes.patch, CRUNCH-97-mafr.patch
>
>
> Crunch core has the following things a package named o.a.c.test and TestCounters class.The source should not contain test package and classes named with Test.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CRUNCH-93) Crunch core source should not
contain packages like o.a.c.test
Posted by "Rahul Sharma (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CRUNCH-93?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13475943#comment-13475943 ]
Rahul Sharma commented on CRUNCH-93:
------------------------------------
yes there is one in the MemCollection, I needed to do this because of the Hadoop API changes from version 1.0.3 to 2.0.alpha. Personally, I do not like to do things with bytecode instrumentation, let me know if you think this can be improved.
> Crunch core source should not contain packages like o.a.c.test
> ---------------------------------------------------------------
>
> Key: CRUNCH-93
> URL: https://issues.apache.org/jira/browse/CRUNCH-93
> Project: Crunch
> Issue Type: Bug
> Reporter: Rahul Sharma
> Priority: Minor
> Attachments: 0001-CRUNCH-93-moving-test-support-classes.patch
>
>
> Crunch core has the following things a package named o.a.c.test and TestCounters class.The source should not contain test package and classes named with Test.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CRUNCH-93) Crunch core source should not
contain packages like o.a.c.test
Posted by "Matthias Friedrich (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CRUNCH-93?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13475893#comment-13475893 ]
Matthias Friedrich commented on CRUNCH-93:
------------------------------------------
I agree with the general plan, too, but I also noticed the javassist trickery and that'll need a closer look as I don't use it often. I'll definitely try to have a look at it next week.
> Crunch core source should not contain packages like o.a.c.test
> ---------------------------------------------------------------
>
> Key: CRUNCH-93
> URL: https://issues.apache.org/jira/browse/CRUNCH-93
> Project: Crunch
> Issue Type: Bug
> Reporter: Rahul Sharma
> Priority: Minor
> Attachments: 0001-CRUNCH-93-moving-test-support-classes.patch
>
>
> Crunch core has the following things a package named o.a.c.test and TestCounters class.The source should not contain test package and classes named with Test.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Updated] (CRUNCH-93) Crunch core source should not contain
packages like o.a.c.test
Posted by "Rahul Sharma (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CRUNCH-93?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Rahul Sharma updated CRUNCH-93:
-------------------------------
Attachment: 0001-CRUNCH-93-moving-test-support-classes.patch
I see that there is lot of testing based code in DoFn and corresponding impls. There are if- else statements in all the methods there. Plus there is this configurationForTest method that is callad from all DoFn impls and MemCollections.
I have removed all of this and created a method to generate TaskIOContext in CrunchTestSupport based on Mockito, which can be used in unit tests. Also created another proxy based TaskIOcontext in MemCollection to pass configuration to all DoFns. Also moved the InMemory emitter to o.a.c.mem.emit package.
> Crunch core source should not contain packages like o.a.c.test
> ---------------------------------------------------------------
>
> Key: CRUNCH-93
> URL: https://issues.apache.org/jira/browse/CRUNCH-93
> Project: Crunch
> Issue Type: Bug
> Reporter: Rahul Sharma
> Priority: Minor
> Attachments: 0001-CRUNCH-93-moving-test-support-classes.patch
>
>
> Crunch core has the following things a package named o.a.c.test and TestCounters class.The source should not contain test package and classes named with Test.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CRUNCH-93) Crunch core source should not
contain packages like o.a.c.test
Posted by "Josh Wills (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CRUNCH-93?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13475867#comment-13475867 ]
Josh Wills commented on CRUNCH-93:
----------------------------------
Okay, I can see the virtues of this. I'd like [~mafr] to weigh in on the details of the patch before we submit it since I'm not an expert here, but I'm +1 for the overall approach.
> Crunch core source should not contain packages like o.a.c.test
> ---------------------------------------------------------------
>
> Key: CRUNCH-93
> URL: https://issues.apache.org/jira/browse/CRUNCH-93
> Project: Crunch
> Issue Type: Bug
> Reporter: Rahul Sharma
> Priority: Minor
> Attachments: 0001-CRUNCH-93-moving-test-support-classes.patch
>
>
> Crunch core has the following things a package named o.a.c.test and TestCounters class.The source should not contain test package and classes named with Test.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CRUNCH-93) Crunch core source should not
contain packages like o.a.c.test
Posted by "Rahul Sharma (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CRUNCH-93?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13473435#comment-13473435 ]
Rahul Sharma commented on CRUNCH-93:
------------------------------------
The package contains 2 classes :
1) InMemoryEmitter : This is used in MemPipeline, maybe we can move it to package o.a.c.impl.mem.emit which is something on line for emitters for MR pipeline
2) TestCounters : These are used in DoFns. I am not sure on this one but the clients are not supposed to use this class directly ? This is used in unit testing only as the context is not there. So shouldn't clients mock and provide a TaskInputOutputContext ? or maybe we can create one in crunch-tests that the clients can use for junit testing ?
> Crunch core source should not contain packages like o.a.c.test
> ---------------------------------------------------------------
>
> Key: CRUNCH-93
> URL: https://issues.apache.org/jira/browse/CRUNCH-93
> Project: Crunch
> Issue Type: Bug
> Reporter: Rahul Sharma
> Priority: Minor
>
> Crunch core has the following things a package named o.a.c.test and TestCounters class.The source should not contain test package and classes named with Test.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Resolved] (CRUNCH-93) Crunch core source should not contain
packages like o.a.c.test
Posted by "Rahul Sharma (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CRUNCH-93?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]
Rahul Sharma resolved CRUNCH-93.
--------------------------------
Resolution: Fixed
Fix Version/s: 0.4.0
Assignee: Rahul Sharma
committed
> Crunch core source should not contain packages like o.a.c.test
> ---------------------------------------------------------------
>
> Key: CRUNCH-93
> URL: https://issues.apache.org/jira/browse/CRUNCH-93
> Project: Crunch
> Issue Type: Bug
> Reporter: Rahul Sharma
> Assignee: Rahul Sharma
> Priority: Minor
> Fix For: 0.4.0
>
> Attachments: 0001-CRUNCH-93-moving-test-support-classes.patch, CRUNCH-97-mafr.patch
>
>
> Crunch core has the following things a package named o.a.c.test and TestCounters class.The source should not contain test package and classes named with Test.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CRUNCH-93) Crunch core source should not
contain packages like o.a.c.test
Posted by "Josh Wills (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CRUNCH-93?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13473561#comment-13473561 ]
Josh Wills commented on CRUNCH-93:
----------------------------------
Re 1, it's also useful for unit testing a DoFn process() implementation by passing it an input, collecting its outputs using the InMemoryEmitter, and checking their contents.
Re 2, TestCounters should be used directly for unit tests, again as a way of verifying that the DoFn you are testing increments the counters you expect it to increment when passed a test input.
> Crunch core source should not contain packages like o.a.c.test
> ---------------------------------------------------------------
>
> Key: CRUNCH-93
> URL: https://issues.apache.org/jira/browse/CRUNCH-93
> Project: Crunch
> Issue Type: Bug
> Reporter: Rahul Sharma
> Priority: Minor
>
> Crunch core has the following things a package named o.a.c.test and TestCounters class.The source should not contain test package and classes named with Test.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CRUNCH-93) Crunch core source should not
contain packages like o.a.c.test
Posted by "Josh Wills (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CRUNCH-93?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13473392#comment-13473392 ]
Josh Wills commented on CRUNCH-93:
----------------------------------
That package has classes that aid in unit testing of Crunch pipelines for clients, which is pretty useful. What do you suggest?
> Crunch core source should not contain packages like o.a.c.test
> ---------------------------------------------------------------
>
> Key: CRUNCH-93
> URL: https://issues.apache.org/jira/browse/CRUNCH-93
> Project: Crunch
> Issue Type: Bug
> Reporter: Rahul Sharma
> Priority: Minor
>
> Crunch core has the following things a package named o.a.c.test and TestCounters class.The source should not contain test package and classes named with Test.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CRUNCH-93) Crunch core source should not
contain packages like o.a.c.test
Posted by "Rahul Sharma (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CRUNCH-93?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13479629#comment-13479629 ]
Rahul Sharma commented on CRUNCH-93:
------------------------------------
thanks Matthias, I will commit it shortly.
> Crunch core source should not contain packages like o.a.c.test
> ---------------------------------------------------------------
>
> Key: CRUNCH-93
> URL: https://issues.apache.org/jira/browse/CRUNCH-93
> Project: Crunch
> Issue Type: Bug
> Reporter: Rahul Sharma
> Priority: Minor
> Attachments: 0001-CRUNCH-93-moving-test-support-classes.patch, CRUNCH-97-mafr.patch
>
>
> Crunch core has the following things a package named o.a.c.test and TestCounters class.The source should not contain test package and classes named with Test.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CRUNCH-93) Crunch core source should not
contain packages like o.a.c.test
Posted by "Rahul Sharma (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CRUNCH-93?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13473927#comment-13473927 ]
Rahul Sharma commented on CRUNCH-93:
------------------------------------
For the Emitter, I think the primary purpose is in MemPipeline. But since it is InMemory so clients can use it in unit tests, this is a side effect rather the intended purpose. Thus I think that it should be in line with emitter for MR pipelines eg. at package o.a.c.impl.mem.emit. This class will be available in crunch-core and clients would be able to unit test their code using it.
As for TestCounters, I am thinking of creating a CrunchTestContext which will contains things that are used for Junit tests eg overriding getCounters methods and provide new TaskAttempID etc. Also move the TestCounters to crunch-test. Clients can use them in JUnit tests in conjunction but they have to depend on crunch-test
I see that both these things are important to unit testing of clients, may be there are other things also which I am not aware of. If possible can we try to move them to crunch-test as these classes provide unit testing support and are not the core.
> Crunch core source should not contain packages like o.a.c.test
> ---------------------------------------------------------------
>
> Key: CRUNCH-93
> URL: https://issues.apache.org/jira/browse/CRUNCH-93
> Project: Crunch
> Issue Type: Bug
> Reporter: Rahul Sharma
> Priority: Minor
>
> Crunch core has the following things a package named o.a.c.test and TestCounters class.The source should not contain test package and classes named with Test.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
[jira] [Commented] (CRUNCH-93) Crunch core source should not
contain packages like o.a.c.test
Posted by "Rahul Sharma (JIRA)" <ji...@apache.org>.
[ https://issues.apache.org/jira/browse/CRUNCH-93?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13474019#comment-13474019 ]
Rahul Sharma commented on CRUNCH-93:
------------------------------------
Sorry to say that CrunchTestContext plan would not work. Just realized that TaskInputOutputContext is an abstract class in hadoop 1.0.3 but the same is an interface in hadoop 2.
> Crunch core source should not contain packages like o.a.c.test
> ---------------------------------------------------------------
>
> Key: CRUNCH-93
> URL: https://issues.apache.org/jira/browse/CRUNCH-93
> Project: Crunch
> Issue Type: Bug
> Reporter: Rahul Sharma
> Priority: Minor
>
> Crunch core has the following things a package named o.a.c.test and TestCounters class.The source should not contain test package and classes named with Test.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira