You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sqoop.apache.org by Jarek Cecho <ja...@apache.org> on 2013/02/18 02:43:32 UTC
Review Request: SQOOP-882 Sqoop2 integration: Auxiliary classes for various
database support
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9487/
-----------------------------------------------------------
Review request for Sqoop.
Description
-------
I've created abstract database handler that can be used in tests to interact with remote databases (create tables, insert data, ...). I've also provided concrete implementation based on Derby. I'll add more handlers later if this patch gets accepted.
This addresses bug SQOOP-882.
https://issues.apache.org/jira/browse/SQOOP-882
Diffs
-----
pom.xml ee013f2c63ff85e4f3b89d75ad860165b0fd0e67
test/pom.xml 66382b6fd7d17d4b345f5960082fb39e2ee100d4
test/src/main/java/org/apache/sqoop/test/db/DatabaseProvider.java PRE-CREATION
test/src/main/java/org/apache/sqoop/test/db/DerbyProvider.java PRE-CREATION
test/src/main/java/org/apache/sqoop/test/minicluster/TomcatSqoopMiniCluster.java 5fa294a2adc4a2f6855fda6e8853009b53554710
test/src/test/java/org/apache/sqoop/integration/TomcatTestCase.java eacf3045580cf93879905b706f1114a407283bad
test/src/test/java/org/apache/sqoop/integration/connector/ConnectorTestCase.java PRE-CREATION
test/src/test/java/org/apache/sqoop/integration/connector/jdbc/generic/TableImportTest.java PRE-CREATION
test/src/test/resources/log4j.properties PRE-CREATION
Diff: https://reviews.apache.org/r/9487/diff/
Testing
-------
I've provided first "real" integration test that this utilizing newly created database handler to set up the environment and move some data. Sadly we currently do not have convenient client API that we can use, so the implementation for connection and job creation is quite "hacky". I'll improve that once we will have reasonable client API.
Thanks,
Jarek Cecho
Re: Review Request: SQOOP-882 Sqoop2 integration: Auxiliary classes for
various database support
Posted by Cheolsoo Park <ch...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9487/#review16968
-----------------------------------------------------------
Have few minor comments.
test/src/main/java/org/apache/sqoop/test/db/DatabaseProvider.java
<https://reviews.apache.org/r/9487/#comment35945>
This is not "DerbyProvider".
test/src/main/java/org/apache/sqoop/test/db/DatabaseProvider.java
<https://reviews.apache.org/r/9487/#comment35979>
thrown -> throw
test/src/main/java/org/apache/sqoop/test/minicluster/TomcatSqoopMiniCluster.java
<https://reviews.apache.org/r/9487/#comment35947>
Please delete this line if not necessary?
test/src/test/java/org/apache/sqoop/integration/TomcatTestCase.java
<https://reviews.apache.org/r/9487/#comment35948>
Doesn't this method always return the correct list of output files? If so, can you change the comment to something like "Return the list of output file names of mapreduce job"?
The word "likely" gives me an impression that it may or may not return the correct result.
test/src/test/java/org/apache/sqoop/integration/TomcatTestCase.java
<https://reviews.apache.org/r/9487/#comment35949>
This can be shorted as follows:
if (!setLines.remove(line)) {
notFound.add(line);
}
- Cheolsoo Park
On Feb. 18, 2013, 1:43 a.m., Jarek Cecho wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9487/
> -----------------------------------------------------------
>
> (Updated Feb. 18, 2013, 1:43 a.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> I've created abstract database handler that can be used in tests to interact with remote databases (create tables, insert data, ...). I've also provided concrete implementation based on Derby. I'll add more handlers later if this patch gets accepted.
>
>
> This addresses bug SQOOP-882.
> https://issues.apache.org/jira/browse/SQOOP-882
>
>
> Diffs
> -----
>
> pom.xml ee013f2c63ff85e4f3b89d75ad860165b0fd0e67
> test/pom.xml 66382b6fd7d17d4b345f5960082fb39e2ee100d4
> test/src/main/java/org/apache/sqoop/test/db/DatabaseProvider.java PRE-CREATION
> test/src/main/java/org/apache/sqoop/test/db/DerbyProvider.java PRE-CREATION
> test/src/main/java/org/apache/sqoop/test/minicluster/TomcatSqoopMiniCluster.java 5fa294a2adc4a2f6855fda6e8853009b53554710
> test/src/test/java/org/apache/sqoop/integration/TomcatTestCase.java eacf3045580cf93879905b706f1114a407283bad
> test/src/test/java/org/apache/sqoop/integration/connector/ConnectorTestCase.java PRE-CREATION
> test/src/test/java/org/apache/sqoop/integration/connector/jdbc/generic/TableImportTest.java PRE-CREATION
> test/src/test/resources/log4j.properties PRE-CREATION
>
> Diff: https://reviews.apache.org/r/9487/diff/
>
>
> Testing
> -------
>
> I've provided first "real" integration test that this utilizing newly created database handler to set up the environment and move some data. Sadly we currently do not have convenient client API that we can use, so the implementation for connection and job creation is quite "hacky". I'll improve that once we will have reasonable client API.
>
>
> Thanks,
>
> Jarek Cecho
>
>
Re: Review Request: SQOOP-882 Sqoop2 integration: Auxiliary classes for
various database support
Posted by Cheolsoo Park <ch...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9487/#review17000
-----------------------------------------------------------
Ship it!
Ship It!
- Cheolsoo Park
On Feb. 23, 2013, 5:16 p.m., Jarek Cecho wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9487/
> -----------------------------------------------------------
>
> (Updated Feb. 23, 2013, 5:16 p.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> I've created abstract database handler that can be used in tests to interact with remote databases (create tables, insert data, ...). I've also provided concrete implementation based on Derby. I'll add more handlers later if this patch gets accepted.
>
>
> This addresses bug SQOOP-882.
> https://issues.apache.org/jira/browse/SQOOP-882
>
>
> Diffs
> -----
>
> pom.xml 5e1b43b63d2e6baccfdf3927d3449d0b5ea189fb
> test/pom.xml 66382b6fd7d17d4b345f5960082fb39e2ee100d4
> test/src/main/java/org/apache/sqoop/test/db/DatabaseProvider.java PRE-CREATION
> test/src/main/java/org/apache/sqoop/test/db/DerbyProvider.java PRE-CREATION
> test/src/main/java/org/apache/sqoop/test/minicluster/TomcatSqoopMiniCluster.java 5fa294a2adc4a2f6855fda6e8853009b53554710
> test/src/test/java/org/apache/sqoop/integration/TomcatTestCase.java eacf3045580cf93879905b706f1114a407283bad
> test/src/test/java/org/apache/sqoop/integration/connector/ConnectorTestCase.java PRE-CREATION
> test/src/test/java/org/apache/sqoop/integration/connector/jdbc/generic/TableImportTest.java PRE-CREATION
> test/src/test/resources/log4j.properties PRE-CREATION
>
> Diff: https://reviews.apache.org/r/9487/diff/
>
>
> Testing
> -------
>
> I've provided first "real" integration test that this utilizing newly created database handler to set up the environment and move some data. Sadly we currently do not have convenient client API that we can use, so the implementation for connection and job creation is quite "hacky". I'll improve that once we will have reasonable client API.
>
>
> Thanks,
>
> Jarek Cecho
>
>
Re: Review Request: SQOOP-882 Sqoop2 integration: Auxiliary classes for
various database support
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9487/
-----------------------------------------------------------
(Updated Feb. 23, 2013, 5:16 p.m.)
Review request for Sqoop.
Changes
-------
I've incorporated Cheolsoo's feedback.
Description
-------
I've created abstract database handler that can be used in tests to interact with remote databases (create tables, insert data, ...). I've also provided concrete implementation based on Derby. I'll add more handlers later if this patch gets accepted.
This addresses bug SQOOP-882.
https://issues.apache.org/jira/browse/SQOOP-882
Diffs (updated)
-----
pom.xml 5e1b43b63d2e6baccfdf3927d3449d0b5ea189fb
test/pom.xml 66382b6fd7d17d4b345f5960082fb39e2ee100d4
test/src/main/java/org/apache/sqoop/test/db/DatabaseProvider.java PRE-CREATION
test/src/main/java/org/apache/sqoop/test/db/DerbyProvider.java PRE-CREATION
test/src/main/java/org/apache/sqoop/test/minicluster/TomcatSqoopMiniCluster.java 5fa294a2adc4a2f6855fda6e8853009b53554710
test/src/test/java/org/apache/sqoop/integration/TomcatTestCase.java eacf3045580cf93879905b706f1114a407283bad
test/src/test/java/org/apache/sqoop/integration/connector/ConnectorTestCase.java PRE-CREATION
test/src/test/java/org/apache/sqoop/integration/connector/jdbc/generic/TableImportTest.java PRE-CREATION
test/src/test/resources/log4j.properties PRE-CREATION
Diff: https://reviews.apache.org/r/9487/diff/
Testing
-------
I've provided first "real" integration test that this utilizing newly created database handler to set up the environment and move some data. Sadly we currently do not have convenient client API that we can use, so the implementation for connection and job creation is quite "hacky". I'll improve that once we will have reasonable client API.
Thanks,
Jarek Cecho