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


Hi David,
thank you very much for publishing the first draft! Overall it looks good to 
me, I have couple of high level notes:

* Can we please remove all the trailing whitespaces? Most of the IDEs have 
ability to do that automatically.

* I see that we are using a lot of Oracle JDBC driver classes from packages 
oracle.sql.* or oracle.jdbc.* thus making the Oracle JDBC driver as a compile 
time dependency. As the Oracle JDBC driver do not have open license, I'm not 
sure this will fly. Is there an option to use the Java JDBC generic structures? 
Or perhaps a reflection?


src/java/org/apache/sqoop/ConnFactory.java
<https://reviews.apache.org/r/18452/#comment66144>

    I would like to eventually see the OraOopManagerFactory merged into the 
DefaultManagerFactory. I'm fine with doing it in follow up JIRA though.


Jarcec

- Jarek Cecho


On Feb. 25, 2014, 3:24 a.m., David Robson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/18452/
> -----------------------------------------------------------
> 
> (Updated Feb. 25, 2014, 3:24 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-1287
>     https://issues.apache.org/jira/browse/SQOOP-1287
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> -------
> 
> Dell Software is contributing an Oracle connector for the Sqoop project.
> This is an initial patch to get early feedback - it is not finished. At the 
> moment it is just the code itself - no tests or documentation.
> There is still more work to do in the code - checkstyle and findbugs has not 
> been resolved as yet.
> 
> 
> Diffs
> -----
> 
>   src/java/org/apache/sqoop/ConnFactory.java 61d3307 
>   src/java/org/apache/sqoop/manager/oracle/OraOopConnManager.java 
> PRE-CREATION 
>   src/java/org/apache/sqoop/manager/oracle/OraOopConstants.java PRE-CREATION 
>   src/java/org/apache/sqoop/manager/oracle/OraOopDBInputSplit.java 
> PRE-CREATION 
>   src/java/org/apache/sqoop/manager/oracle/OraOopDBRecordReader.java 
> PRE-CREATION 
>   src/java/org/apache/sqoop/manager/oracle/OraOopDataDrivenDBInputFormat.java 
> PRE-CREATION 
>   src/java/org/apache/sqoop/manager/oracle/OraOopGenerics.java PRE-CREATION 
>   src/java/org/apache/sqoop/manager/oracle/OraOopJdbcUrl.java PRE-CREATION 
>   src/java/org/apache/sqoop/manager/oracle/OraOopLog.java PRE-CREATION 
>   src/java/org/apache/sqoop/manager/oracle/OraOopLogFactory.java PRE-CREATION 
>   src/java/org/apache/sqoop/manager/oracle/OraOopLogMessage.java PRE-CREATION 
>   src/java/org/apache/sqoop/manager/oracle/OraOopManagerFactory.java 
> PRE-CREATION 
>   src/java/org/apache/sqoop/manager/oracle/OraOopOracleDataChunk.java 
> PRE-CREATION 
>   src/java/org/apache/sqoop/manager/oracle/OraOopOracleDataChunkExtent.java 
> PRE-CREATION 
>   
> src/java/org/apache/sqoop/manager/oracle/OraOopOracleDataChunkPartition.java 
> PRE-CREATION 
>   src/java/org/apache/sqoop/manager/oracle/OraOopOracleQueries.java 
> PRE-CREATION 
>   src/java/org/apache/sqoop/manager/oracle/OraOopOutputFormatBase.java 
> PRE-CREATION 
>   src/java/org/apache/sqoop/manager/oracle/OraOopOutputFormatInsert.java 
> PRE-CREATION 
>   src/java/org/apache/sqoop/manager/oracle/OraOopOutputFormatUpdate.java 
> PRE-CREATION 
>   src/java/org/apache/sqoop/manager/oracle/OraOopUtilities.java PRE-CREATION 
>   src/java/org/apache/sqoop/manager/oracle/OracleActiveInstance.java 
> PRE-CREATION 
>   src/java/org/apache/sqoop/manager/oracle/OracleConnectionFactory.java 
> PRE-CREATION 
>   src/java/org/apache/sqoop/manager/oracle/OracleTable.java PRE-CREATION 
>   src/java/org/apache/sqoop/manager/oracle/OracleTableColumn.java 
> PRE-CREATION 
>   src/java/org/apache/sqoop/manager/oracle/OracleTableColumns.java 
> PRE-CREATION 
>   src/java/org/apache/sqoop/manager/oracle/OracleTablePartition.java 
> PRE-CREATION 
>   src/java/org/apache/sqoop/manager/oracle/OracleTablePartitions.java 
> PRE-CREATION 
>   src/java/org/apache/sqoop/manager/oracle/OracleVersion.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/18452/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> David Robson
> 
>

Reply via email to