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 2015/03/20 02:32:43 UTC
Review Request 32275: SQOOP-2246 Sqoop2: Generic JDBC: Use
jdbcProperties
when creating database connection in GenericJDBCExecutor
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32275/
-----------------------------------------------------------
Review request for Sqoop.
Bugs: SQOOP-2246
https://issues.apache.org/jira/browse/SQOOP-2246
Repository: sqoop-sqoop2
Description
-------
I've updated the Executor constructor to take entire link configuration object rather then few specific arguments. This way any new added configuration options will be immediatelly available in the executor without need to add another parameter to the constructor.
Diffs
-----
connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExecutor.java 5af34a5
connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExtractor.java 3287e16
connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcFromInitializer.java 6ad2cab
connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcLoader.java ab1ac86
connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcToDestroyer.java e381651
connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcToInitializer.java 400c0f2
connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/GenericJdbcExecutorTest.java c3b8171
connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/GenericJdbcTestConstants.java 67ba5bf
connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestExtractor.java 803d37b
connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestFromInitializer.java e9c8d41
connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestLoader.java 2479f89
connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestToInitializer.java a61de7d
Diff: https://reviews.apache.org/r/32275/diff/
Testing
-------
All tests seems to be passing.
Thanks,
Jarek Cecho
Re: Review Request 32275: SQOOP-2246 Sqoop2: Generic JDBC: Use
jdbcProperties when creating database connection in GenericJDBCExecutor
Posted by Abraham Elmahrek <ab...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32275/#review77487
-----------------------------------------------------------
Ship it!
Ship It!
- Abraham Elmahrek
On March 20, 2015, 1:32 a.m., Jarek Cecho wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32275/
> -----------------------------------------------------------
>
> (Updated March 20, 2015, 1:32 a.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-2246
> https://issues.apache.org/jira/browse/SQOOP-2246
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> I've updated the Executor constructor to take entire link configuration object rather then few specific arguments. This way any new added configuration options will be immediatelly available in the executor without need to add another parameter to the constructor.
>
>
> Diffs
> -----
>
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExecutor.java 5af34a5
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExtractor.java 3287e16
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcFromInitializer.java 6ad2cab
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcLoader.java ab1ac86
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcToDestroyer.java e381651
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcToInitializer.java 400c0f2
> connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/GenericJdbcExecutorTest.java c3b8171
> connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/GenericJdbcTestConstants.java 67ba5bf
> connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestExtractor.java 803d37b
> connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestFromInitializer.java e9c8d41
> connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestLoader.java 2479f89
> connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestToInitializer.java a61de7d
>
> Diff: https://reviews.apache.org/r/32275/diff/
>
>
> Testing
> -------
>
> All tests seems to be passing.
>
>
> Thanks,
>
> Jarek Cecho
>
>
Re: Review Request 32275: SQOOP-2246 Sqoop2: Generic JDBC: Use
jdbcProperties when creating database connection in GenericJDBCExecutor
Posted by Jarek Cecho <ja...@apache.org>.
> On March 23, 2015, 10:19 p.m., Abraham Elmahrek wrote:
> > connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExecutor.java, line 83
> > <https://reviews.apache.org/r/32275/diff/1/?file=901042#file901042line83>
> >
> > The same JVM instance might load the class twice? In some JDBC drivers, they do some static initialization. Doing that twice can cause problems. I've seen this when testing MySQL to PostgreSQL and vice versa.
Thanks for bringing the point up. The ClassUtils.loadClass will call Class.forName() underneath and I believe that it's a requirement for ClassLoaders to load one class only once (as long as we have one class loader). I know that Tomcat is doing a lot of magic with classloaders, but I believe that entire app (war application) do have the shared classloader. I might be wrong though.
- Jarek
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32275/#review77479
-----------------------------------------------------------
On March 20, 2015, 1:32 a.m., Jarek Cecho wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32275/
> -----------------------------------------------------------
>
> (Updated March 20, 2015, 1:32 a.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-2246
> https://issues.apache.org/jira/browse/SQOOP-2246
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> I've updated the Executor constructor to take entire link configuration object rather then few specific arguments. This way any new added configuration options will be immediatelly available in the executor without need to add another parameter to the constructor.
>
>
> Diffs
> -----
>
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExecutor.java 5af34a5
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExtractor.java 3287e16
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcFromInitializer.java 6ad2cab
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcLoader.java ab1ac86
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcToDestroyer.java e381651
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcToInitializer.java 400c0f2
> connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/GenericJdbcExecutorTest.java c3b8171
> connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/GenericJdbcTestConstants.java 67ba5bf
> connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestExtractor.java 803d37b
> connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestFromInitializer.java e9c8d41
> connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestLoader.java 2479f89
> connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestToInitializer.java a61de7d
>
> Diff: https://reviews.apache.org/r/32275/diff/
>
>
> Testing
> -------
>
> All tests seems to be passing.
>
>
> Thanks,
>
> Jarek Cecho
>
>
Re: Review Request 32275: SQOOP-2246 Sqoop2: Generic JDBC: Use
jdbcProperties when creating database connection in GenericJDBCExecutor
Posted by Abraham Elmahrek <ab...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/32275/#review77479
-----------------------------------------------------------
connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExecutor.java
<https://reviews.apache.org/r/32275/#comment125566>
The same JVM instance might load the class twice? In some JDBC drivers, they do some static initialization. Doing that twice can cause problems. I've seen this when testing MySQL to PostgreSQL and vice versa.
connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExecutor.java
<https://reviews.apache.org/r/32275/#comment125567>
Awesome!
connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExecutor.java
<https://reviews.apache.org/r/32275/#comment125568>
Kind of a new feature I'd say. We never accepted passwordless login in the past. Cool!
- Abraham Elmahrek
On March 20, 2015, 1:32 a.m., Jarek Cecho wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/32275/
> -----------------------------------------------------------
>
> (Updated March 20, 2015, 1:32 a.m.)
>
>
> Review request for Sqoop.
>
>
> Bugs: SQOOP-2246
> https://issues.apache.org/jira/browse/SQOOP-2246
>
>
> Repository: sqoop-sqoop2
>
>
> Description
> -------
>
> I've updated the Executor constructor to take entire link configuration object rather then few specific arguments. This way any new added configuration options will be immediatelly available in the executor without need to add another parameter to the constructor.
>
>
> Diffs
> -----
>
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExecutor.java 5af34a5
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcExtractor.java 3287e16
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcFromInitializer.java 6ad2cab
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcLoader.java ab1ac86
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcToDestroyer.java e381651
> connector/connector-generic-jdbc/src/main/java/org/apache/sqoop/connector/jdbc/GenericJdbcToInitializer.java 400c0f2
> connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/GenericJdbcExecutorTest.java c3b8171
> connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/GenericJdbcTestConstants.java 67ba5bf
> connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestExtractor.java 803d37b
> connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestFromInitializer.java e9c8d41
> connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestLoader.java 2479f89
> connector/connector-generic-jdbc/src/test/java/org/apache/sqoop/connector/jdbc/TestToInitializer.java a61de7d
>
> Diff: https://reviews.apache.org/r/32275/diff/
>
>
> Testing
> -------
>
> All tests seems to be passing.
>
>
> Thanks,
>
> Jarek Cecho
>
>