sqoop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Abraham Elmahrek" <...@cloudera.com>
Subject Re: Review Request 10964: SQOOP-927 Sqoop2: Integration: Mapreduce specific tests should be running on MiniCluster
Date Wed, 10 Jul 2013 23:27:39 GMT


> On July 9, 2013, 6:14 p.m., Jarek Cecho wrote:
> > test/src/main/java/org/apache/sqoop/test/hadoop/HadoopStandaloneCluster.java, line
23
> > <https://reviews.apache.org/r/10964/diff/4/?file=318881#file318881line23>
> >
> >     Nit: I'm afraid that the word "cluster" in the name can be quite confusing as
this is not using any cluster at all. Would it be possible to rename it to something like
LocalMode or LocalRunner?

I changed every thing to be suffixed with the word "Runner". I feel as though this overlaps
with MapReduce nomenclature. Any thoughts?


> On July 9, 2013, 6:14 p.m., Jarek Cecho wrote:
> > test/src/main/java/org/apache/sqoop/test/hadoop/HadoopMiniCluster.java, lines 50-51
> > <https://reviews.apache.org/r/10964/diff/4/?file=318880#file318880line50>
> >
> >     The getDataDir seems to be overloaded. Here it's used for local files that are
generated by the MiniClusters, however we are also using it for the HDFS files. I would recommend
to distinguish those two usages.

Added 'getTestDirectory' to symbolically discern between data directory and test directory.


> On July 9, 2013, 6:14 p.m., Jarek Cecho wrote:
> > test/src/main/java/org/apache/sqoop/test/testcases/TomcatTestCase.java, lines 62-75
> > <https://reviews.apache.org/r/10964/diff/4/?file=318883#file318883line62>
> >
> >     I think that the main temporary path should be owned by this class because we
are storing there a lot of information required for the Sqoop Server itself (logs, metastore,
configuration, ....). Those files exists outside of the mapreduce/hdfs.

Doesn't it make more sense for the sqoop minicluster classes to manage their own temporary
path? It seems like we should only provide a base directory.


> On July 9, 2013, 6:14 p.m., Jarek Cecho wrote:
> > test/src/main/java/org/apache/sqoop/test/testcases/ConnectorTestCase.java, lines
52-54
> > <https://reviews.apache.org/r/10964/diff/4/?file=318882#file318882line52>
> >
> >     I think that we should be able to put the HadoopCluster initialization into
method annotated with @BeforeClass and override this method in child if necessary. This way
we should save some time bootstrapping the Minicluster for test cases that have more then
one test method.

Works!


- Abraham


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


On July 8, 2013, 8:47 p.m., Abraham Elmahrek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10964/
> -----------------------------------------------------------
> 
> (Updated July 8, 2013, 8:47 p.m.)
> 
> 
> Review request for Sqoop and Jarek Cecho.
> 
> 
> Bugs: SQOOP-927
>     https://issues.apache.org/jira/browse/SQOOP-927
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> commit ff7dba55a09dd7789a34136233000c625759e583
> Author: Abraham Elmahrek <abraham@elmahrek.com>
> Date:   Fri Apr 26 15:10:24 2013 -0700
> 
>     SQOOP-927 Sqoop2: Integration: Mapreduce specific tests should be running on MiniCluster
>     
>     Handle MiniDFSCluster and MiniMRClientCluster on own.
>     
>     Set yarn.application.classpath to get over classpath errors.
>     Set to use fair scheduler.
> 
> :100644 100644 0abbb18... f09704b... M	pom.xml
> :100644 100644 6eb3184... a4c2a5b... M	test/src/test/java/org/apache/sqoop/integration/TomcatTestCase.java
> :100644 100644 0f48a8b... 758c885... M	test/src/test/java/org/apache/sqoop/integration/server/VersionTest.java
> 
> 
> Diffs
> -----
> 
>   pom.xml 8785e01000cc6e7d5a74ffeb96b83d236a657df8 
>   test/src/main/java/org/apache/sqoop/test/asserts/HdfsAsserts.java 056e6124b918ce5821c389e388a0cdfa68fd7fc0

>   test/src/main/java/org/apache/sqoop/test/hadoop/HadoopCluster.java PRE-CREATION 
>   test/src/main/java/org/apache/sqoop/test/hadoop/HadoopClusterFactory.java PRE-CREATION

>   test/src/main/java/org/apache/sqoop/test/hadoop/HadoopMiniCluster.java PRE-CREATION

>   test/src/main/java/org/apache/sqoop/test/hadoop/HadoopStandaloneCluster.java PRE-CREATION

>   test/src/main/java/org/apache/sqoop/test/testcases/ConnectorTestCase.java 6aeadd4e54e47e5644270b15813b2d9c4cedc059

>   test/src/main/java/org/apache/sqoop/test/testcases/TomcatTestCase.java ca77e64486d80b38306b5b30e185e6278ad7eaf1

>   test/src/main/java/org/apache/sqoop/test/utils/HdfsUtils.java 95dd177a11c5d822e35ca54e5d93785f0a40fbfc

> 
> Diff: https://reviews.apache.org/r/10964/diff/
> 
> 
> Testing
> -------
> 
> Ran integration tests without and with miniclusters.
> Currently need to use both miniclusters or neither for tests to work.
> 
> 
> Thanks,
> 
> Abraham Elmahrek
> 
>


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