Home | About | Sematext search-lucene.com search-hadoop.com
 Search Hadoop and all its subprojects:

Switch to Threaded View
Sqoop, mail # dev - Re: Review Request 10964: SQOOP-927 Sqoop2: Integration: Mapreduce specific tests should be running on MiniCluster


Copy link to this message
-
Re: Review Request 10964: SQOOP-927 Sqoop2: Integration: Mapreduce specific tests should be running on MiniCluster
Jarek Cecho 2013-07-11, 20:58


> 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.
>
> Abraham Elmahrek wrote:
>     Added 'getTestDirectory' to symbolically discern between data directory and test directory.

Thank you!
> 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?
>
> Abraham Elmahrek wrote:
>     I changed every thing to be suffixed with the word "Runner". I feel as though this overlaps with MapReduce nomenclature. Any thoughts?

I do not personally mind word Runner, but some other ideas:

* "Implementation" -> LocalImplementation, MiniClusterImplementation, RealClusterImplementation
* "Impl" -> LocalImpl, MiniClusterImpl, RealClusterImpl
> 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.
>
> Abraham Elmahrek wrote:
>     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.

The tmpPath in this case is actually a base directory from this perspective. Child test cases are expected to use this, append additional sub directory and use it. It's not called "base" though because we already have one base path that is shared for all test cases, this one is specific for each particular testing method.
- Jarek
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/10964/#review22919
-----------------------------------------------------------
On July 10, 2013, 11:27 p.m., Abraham Elmahrek wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/10964/
> -----------------------------------------------------------
>
> (Updated July 10, 2013, 11:27 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 <[EMAIL PROTECTED]>
> 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