You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sqoop.apache.org by Eric Wadsworth <er...@wadhome.org> on 2011/12/09 22:41:49 UTC

Review Request: Added a ConfigurationHolder class, which adapts the older hadoop Configuration with the needs of sqoop.

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

Review request for Sqoop and Jarek Cecho.


Summary
-------

I added a ConfigurationHolder class, to try and make sqoop more compatible with hadoop 0.20.x.


This addresses bug https://issues.apache.org/jira/browse/SQOOP-384.
    https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/SQOOP-384


Diffs
-----

  /src/java/com/cloudera/sqoop/ConnFactory.java 1212018 
  /src/java/com/cloudera/sqoop/Sqoop.java 1212018 
  /src/java/com/cloudera/sqoop/SqoopOptions.java 1212018 
  /src/java/com/cloudera/sqoop/config/ConfigurationHelper.java 1212018 
  /src/java/com/cloudera/sqoop/hive/HiveImport.java 1212018 
  /src/java/com/cloudera/sqoop/hive/TableDefWriter.java 1212018 
  /src/java/com/cloudera/sqoop/io/CodecMap.java 1212018 
  /src/java/com/cloudera/sqoop/io/LobFile.java 1212018 
  /src/java/com/cloudera/sqoop/io/LobReaderCache.java 1212018 
  /src/java/com/cloudera/sqoop/io/SplittingOutputStream.java 1212018 
  /src/java/com/cloudera/sqoop/lib/LargeObjectLoader.java 1212018 
  /src/java/com/cloudera/sqoop/manager/MySQLUtils.java 1212018 
  /src/java/com/cloudera/sqoop/mapreduce/AvroJob.java 1212018 
  /src/java/com/cloudera/sqoop/mapreduce/ExportJobBase.java 1212018 
  /src/java/com/cloudera/sqoop/mapreduce/MySQLDumpMapper.java 1212018 
  /src/java/com/cloudera/sqoop/mapreduce/db/DBConfiguration.java 1212018 
  /src/java/com/cloudera/sqoop/mapreduce/db/DBRecordReader.java 1212018 
  /src/java/com/cloudera/sqoop/mapreduce/db/DataDrivenDBInputFormat.java 1212018 
  /src/java/com/cloudera/sqoop/mapreduce/db/DataDrivenDBRecordReader.java 1212018 
  /src/java/com/cloudera/sqoop/mapreduce/db/OracleDBRecordReader.java 1212018 
  /src/java/com/cloudera/sqoop/mapreduce/db/OracleDataDrivenDBRecordReader.java 1212018 
  /src/java/com/cloudera/sqoop/metastore/JobStorageFactory.java 1212018 
  /src/java/com/cloudera/sqoop/metastore/hsqldb/HsqldbMetaStore.java 1212018 
  /src/java/com/cloudera/sqoop/util/DirectImportUtils.java 1212018 
  /src/java/com/cloudera/sqoop/util/TaskId.java 1212018 
  /src/java/org/apache/sqoop/ConnFactory.java 1212018 
  /src/java/org/apache/sqoop/Sqoop.java 1212018 
  /src/java/org/apache/sqoop/SqoopOptions.java 1212018 
  /src/java/org/apache/sqoop/config/ConfigurationHelper.java 1212018 
  /src/java/org/apache/sqoop/config/ConfigurationHolder.java PRE-CREATION 
  /src/java/org/apache/sqoop/hbase/HBasePutProcessor.java 1212018 
  /src/java/org/apache/sqoop/hive/HiveImport.java 1212018 
  /src/java/org/apache/sqoop/hive/TableDefWriter.java 1212018 
  /src/java/org/apache/sqoop/io/CodecMap.java 1212018 
  /src/java/org/apache/sqoop/io/LobFile.java 1212018 
  /src/java/org/apache/sqoop/io/LobReaderCache.java 1212018 
  /src/java/org/apache/sqoop/io/SplittingOutputStream.java 1212018 
  /src/java/org/apache/sqoop/lib/LargeObjectLoader.java 1212018 
  /src/java/org/apache/sqoop/lib/LobRef.java 1212018 
  /src/java/org/apache/sqoop/manager/DirectPostgresqlManager.java 1212018 
  /src/java/org/apache/sqoop/manager/MySQLUtils.java 1212018 
  /src/java/org/apache/sqoop/mapreduce/AsyncSqlRecordWriter.java 1212018 
  /src/java/org/apache/sqoop/mapreduce/AutoProgressMapper.java 1212018 
  /src/java/org/apache/sqoop/mapreduce/AvroExportMapper.java 1212018 
  /src/java/org/apache/sqoop/mapreduce/AvroImportMapper.java 1212018 
  /src/java/org/apache/sqoop/mapreduce/AvroJob.java 1212018 
  /src/java/org/apache/sqoop/mapreduce/AvroOutputFormat.java 1212018 
  /src/java/org/apache/sqoop/mapreduce/AvroRecordReader.java 1212018 
  /src/java/org/apache/sqoop/mapreduce/CombineShimRecordReader.java 1212018 
  /src/java/org/apache/sqoop/mapreduce/DataDrivenImportJob.java 1212018 
  /src/java/org/apache/sqoop/mapreduce/DelegatingOutputFormat.java 1212018 
  /src/java/org/apache/sqoop/mapreduce/ExportJobBase.java 1212018 
  /src/java/org/apache/sqoop/mapreduce/ExportOutputFormat.java 1212018 
  /src/java/org/apache/sqoop/mapreduce/HBaseImportJob.java 1212018 
  /src/java/org/apache/sqoop/mapreduce/ImportJobBase.java 1212018 
  /src/java/org/apache/sqoop/mapreduce/JobBase.java 1212018 
  /src/java/org/apache/sqoop/mapreduce/MergeJob.java 1212018 
  /src/java/org/apache/sqoop/mapreduce/MergeMapperBase.java 1212018 
  /src/java/org/apache/sqoop/mapreduce/MergeRecord.java 1212018 
  /src/java/org/apache/sqoop/mapreduce/MergeTextMapper.java 1212018 
  /src/java/org/apache/sqoop/mapreduce/MySQLDumpImportJob.java 1212018 
  /src/java/org/apache/sqoop/mapreduce/MySQLDumpMapper.java 1212018 
  /src/java/org/apache/sqoop/mapreduce/MySQLExportJob.java 1212018 
  /src/java/org/apache/sqoop/mapreduce/MySQLExportMapper.java 1212018 
  /src/java/org/apache/sqoop/mapreduce/RawKeyTextOutputFormat.java 1212018 
  /src/java/org/apache/sqoop/mapreduce/TextExportMapper.java 1212018 
  /src/java/org/apache/sqoop/mapreduce/UpdateOutputFormat.java 1212018 
  /src/java/org/apache/sqoop/mapreduce/db/BigDecimalSplitter.java 1212018 
  /src/java/org/apache/sqoop/mapreduce/db/BooleanSplitter.java 1212018 
  /src/java/org/apache/sqoop/mapreduce/db/DBConfiguration.java 1212018 
  /src/java/org/apache/sqoop/mapreduce/db/DBInputFormat.java 1212018 
  /src/java/org/apache/sqoop/mapreduce/db/DBRecordReader.java 1212018 
  /src/java/org/apache/sqoop/mapreduce/db/DBSplitter.java 1212018 
  /src/java/org/apache/sqoop/mapreduce/db/DataDrivenDBInputFormat.java 1212018 
  /src/java/org/apache/sqoop/mapreduce/db/DataDrivenDBRecordReader.java 1212018 
  /src/java/org/apache/sqoop/mapreduce/db/DateSplitter.java 1212018 
  /src/java/org/apache/sqoop/mapreduce/db/FloatSplitter.java 1212018 
  /src/java/org/apache/sqoop/mapreduce/db/IntegerSplitter.java 1212018 
  /src/java/org/apache/sqoop/mapreduce/db/OracleDBRecordReader.java 1212018 
  /src/java/org/apache/sqoop/mapreduce/db/OracleDataDrivenDBInputFormat.java 1212018 
  /src/java/org/apache/sqoop/mapreduce/db/OracleDataDrivenDBRecordReader.java 1212018 
  /src/java/org/apache/sqoop/mapreduce/db/TextSplitter.java 1212018 
  /src/java/org/apache/sqoop/metastore/JobStorageFactory.java 1212018 
  /src/java/org/apache/sqoop/metastore/hsqldb/AutoHsqldbStorage.java 1212018 
  /src/java/org/apache/sqoop/metastore/hsqldb/HsqldbJobStorage.java 1212018 
  /src/java/org/apache/sqoop/metastore/hsqldb/HsqldbMetaStore.java 1212018 
  /src/java/org/apache/sqoop/tool/BaseSqoopTool.java 1212018 
  /src/java/org/apache/sqoop/tool/CodeGenTool.java 1212018 
  /src/java/org/apache/sqoop/tool/CreateHiveTableTool.java 1212018 
  /src/java/org/apache/sqoop/tool/ImportAllTablesTool.java 1212018 
  /src/java/org/apache/sqoop/tool/ImportTool.java 1212018 
  /src/java/org/apache/sqoop/tool/JobTool.java 1212018 
  /src/java/org/apache/sqoop/tool/MetastoreTool.java 1212018 
  /src/java/org/apache/sqoop/tool/SqoopTool.java 1212018 
  /src/java/org/apache/sqoop/util/AppendUtils.java 1212018 
  /src/java/org/apache/sqoop/util/DirectImportUtils.java 1212018 
  /src/java/org/apache/sqoop/util/TaskId.java 1212018 
  /src/test/com/cloudera/sqoop/TestIncrementalImport.java 1212018 
  /src/test/com/cloudera/sqoop/TestMerge.java 1212018 
  /src/test/com/cloudera/sqoop/lib/TestBlobRef.java 1212018 
  /src/test/com/cloudera/sqoop/lib/TestClobRef.java 1212018 
  /src/test/com/cloudera/sqoop/lib/TestLargeObjectLoader.java 1212018 
  /src/test/com/cloudera/sqoop/mapreduce/TestImportJob.java 1212018 
  /src/test/com/cloudera/sqoop/testutil/ImportJobTestCase.java 1212018 
  /src/test/com/cloudera/sqoop/testutil/InjectableConnManager.java 1212018 
  /src/test/com/cloudera/sqoop/tool/TestToolPlugin.java 1212018 

Diff: https://reviews.apache.org/r/3126/diff


Testing
-------

"ant test" passes. I haven't had much success running sqoop at all, due to the bug this patch is trying to fix (SQOOP-384).


Thanks,

Eric


Re: Review Request: Added a ConfigurationHolder class, which adapts the older hadoop Configuration with the needs of sqoop.

Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3126/#review3969
-----------------------------------------------------------

Ship it!


Hi Eric,
changes looks good. Can you please upload your patch to SQOOP-412 instead of SQOOP-384? It's sub issue created for covering only the changes for getting getInstances working.

Jarcec

- Jarek


On 2011-12-12 19:31:05, Eric Wadsworth wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3126/
> -----------------------------------------------------------
> 
> (Updated 2011-12-12 19:31:05)
> 
> 
> Review request for Sqoop and Jarek Cecho.
> 
> 
> Summary
> -------
> 
> I added a ConfigurationHolder class, to try and make sqoop more compatible with hadoop 0.20.x.
> 
> 
> This addresses bug https://issues.apache.org/jira/browse/SQOOP-384.
>     https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/SQOOP-384
> 
> 
> Diffs
> -----
> 
>   /src/java/org/apache/sqoop/config/ConfigurationHelper.java 1213348 
>   /src/java/org/apache/sqoop/metastore/JobStorageFactory.java 1213348 
>   /src/java/org/apache/sqoop/tool/SqoopTool.java 1213348 
> 
> Diff: https://reviews.apache.org/r/3126/diff
> 
> 
> Testing
> -------
> 
> "ant test" passes. I haven't had much success running sqoop at all, due to the bug this patch is trying to fix (SQOOP-384).
> 
> 
> Thanks,
> 
> Eric
> 
>


Re: Review Request: Added a ConfigurationHolder class, which adapts the older hadoop Configuration with the needs of sqoop.

Posted by Eric Wadsworth <er...@wadhome.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3126/
-----------------------------------------------------------

(Updated 2011-12-12 19:31:05.321849)


Review request for Sqoop and Jarek Cecho.


Changes
-------

This only contains a static method on ConfigurationHelper to provide functionality for getInstances(), along with a couple of changes in code to use it.


Summary
-------

I added a ConfigurationHolder class, to try and make sqoop more compatible with hadoop 0.20.x.


This addresses bug https://issues.apache.org/jira/browse/SQOOP-384.
    https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/SQOOP-384


Diffs (updated)
-----

  /src/java/org/apache/sqoop/config/ConfigurationHelper.java 1213348 
  /src/java/org/apache/sqoop/metastore/JobStorageFactory.java 1213348 
  /src/java/org/apache/sqoop/tool/SqoopTool.java 1213348 

Diff: https://reviews.apache.org/r/3126/diff


Testing
-------

"ant test" passes. I haven't had much success running sqoop at all, due to the bug this patch is trying to fix (SQOOP-384).


Thanks,

Eric


Re: Review Request: Added a ConfigurationHolder class, which adapts the older hadoop Configuration with the needs of sqoop.

Posted by Eric Wadsworth <er...@wadhome.org>.

> On 2011-12-10 19:49:02, Jarek Cecho wrote:
> > Hi Eric,
> > thank you very much for your time and effort that you've spent on this issue. It looks like you did a lot of changes across entire sqoop code base.
> > 
> > The patch seems to me pretty heavy for just wrapping one method that is used only twice in sqoop code base. Instead of wrapping Configuration class into own object and then doing all the changes, I would propose to create our own method getInstances and put it somewhere (ConfigurationHelper seems as a good candidate) as a static method with following definition getInstances(Configuration conf, String name, Class<U> xface). This method would firstly test configuration object whether it defines method getInstances using JAVA reflection and if so, it would call the default implementation that is present in hadoop 0.21+ or CHD3. If not we would execute our own code that would simulate the default behavior for hadoop.
> > 
> > What do you think about that?
> > 
> > Jarcec

Jarcec,

Yes, that would work. I usually try to avoid using reflection whenever possible (it makes refactoring a nightmare), but perhaps in this case it is preferable to the wrapping solution, since it's such a lighter touch. I'll see if I can get a patch out tomorrow that does this.

--- wad


- Eric


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


On 2011-12-09 21:41:48, Eric Wadsworth wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3126/
> -----------------------------------------------------------
> 
> (Updated 2011-12-09 21:41:48)
> 
> 
> Review request for Sqoop and Jarek Cecho.
> 
> 
> Summary
> -------
> 
> I added a ConfigurationHolder class, to try and make sqoop more compatible with hadoop 0.20.x.
> 
> 
> This addresses bug https://issues.apache.org/jira/browse/SQOOP-384.
>     https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/SQOOP-384
> 
> 
> Diffs
> -----
> 
>   /src/java/com/cloudera/sqoop/ConnFactory.java 1212018 
>   /src/java/com/cloudera/sqoop/Sqoop.java 1212018 
>   /src/java/com/cloudera/sqoop/SqoopOptions.java 1212018 
>   /src/java/com/cloudera/sqoop/config/ConfigurationHelper.java 1212018 
>   /src/java/com/cloudera/sqoop/hive/HiveImport.java 1212018 
>   /src/java/com/cloudera/sqoop/hive/TableDefWriter.java 1212018 
>   /src/java/com/cloudera/sqoop/io/CodecMap.java 1212018 
>   /src/java/com/cloudera/sqoop/io/LobFile.java 1212018 
>   /src/java/com/cloudera/sqoop/io/LobReaderCache.java 1212018 
>   /src/java/com/cloudera/sqoop/io/SplittingOutputStream.java 1212018 
>   /src/java/com/cloudera/sqoop/lib/LargeObjectLoader.java 1212018 
>   /src/java/com/cloudera/sqoop/manager/MySQLUtils.java 1212018 
>   /src/java/com/cloudera/sqoop/mapreduce/AvroJob.java 1212018 
>   /src/java/com/cloudera/sqoop/mapreduce/ExportJobBase.java 1212018 
>   /src/java/com/cloudera/sqoop/mapreduce/MySQLDumpMapper.java 1212018 
>   /src/java/com/cloudera/sqoop/mapreduce/db/DBConfiguration.java 1212018 
>   /src/java/com/cloudera/sqoop/mapreduce/db/DBRecordReader.java 1212018 
>   /src/java/com/cloudera/sqoop/mapreduce/db/DataDrivenDBInputFormat.java 1212018 
>   /src/java/com/cloudera/sqoop/mapreduce/db/DataDrivenDBRecordReader.java 1212018 
>   /src/java/com/cloudera/sqoop/mapreduce/db/OracleDBRecordReader.java 1212018 
>   /src/java/com/cloudera/sqoop/mapreduce/db/OracleDataDrivenDBRecordReader.java 1212018 
>   /src/java/com/cloudera/sqoop/metastore/JobStorageFactory.java 1212018 
>   /src/java/com/cloudera/sqoop/metastore/hsqldb/HsqldbMetaStore.java 1212018 
>   /src/java/com/cloudera/sqoop/util/DirectImportUtils.java 1212018 
>   /src/java/com/cloudera/sqoop/util/TaskId.java 1212018 
>   /src/java/org/apache/sqoop/ConnFactory.java 1212018 
>   /src/java/org/apache/sqoop/Sqoop.java 1212018 
>   /src/java/org/apache/sqoop/SqoopOptions.java 1212018 
>   /src/java/org/apache/sqoop/config/ConfigurationHelper.java 1212018 
>   /src/java/org/apache/sqoop/config/ConfigurationHolder.java PRE-CREATION 
>   /src/java/org/apache/sqoop/hbase/HBasePutProcessor.java 1212018 
>   /src/java/org/apache/sqoop/hive/HiveImport.java 1212018 
>   /src/java/org/apache/sqoop/hive/TableDefWriter.java 1212018 
>   /src/java/org/apache/sqoop/io/CodecMap.java 1212018 
>   /src/java/org/apache/sqoop/io/LobFile.java 1212018 
>   /src/java/org/apache/sqoop/io/LobReaderCache.java 1212018 
>   /src/java/org/apache/sqoop/io/SplittingOutputStream.java 1212018 
>   /src/java/org/apache/sqoop/lib/LargeObjectLoader.java 1212018 
>   /src/java/org/apache/sqoop/lib/LobRef.java 1212018 
>   /src/java/org/apache/sqoop/manager/DirectPostgresqlManager.java 1212018 
>   /src/java/org/apache/sqoop/manager/MySQLUtils.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/AsyncSqlRecordWriter.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/AutoProgressMapper.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/AvroExportMapper.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/AvroImportMapper.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/AvroJob.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/AvroOutputFormat.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/AvroRecordReader.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/CombineShimRecordReader.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/DataDrivenImportJob.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/DelegatingOutputFormat.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/ExportJobBase.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/ExportOutputFormat.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/HBaseImportJob.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/ImportJobBase.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/JobBase.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/MergeJob.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/MergeMapperBase.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/MergeRecord.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/MergeTextMapper.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/MySQLDumpImportJob.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/MySQLDumpMapper.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/MySQLExportJob.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/MySQLExportMapper.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/RawKeyTextOutputFormat.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/TextExportMapper.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/UpdateOutputFormat.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/db/BigDecimalSplitter.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/db/BooleanSplitter.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/db/DBConfiguration.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/db/DBInputFormat.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/db/DBRecordReader.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/db/DBSplitter.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/db/DataDrivenDBInputFormat.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/db/DataDrivenDBRecordReader.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/db/DateSplitter.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/db/FloatSplitter.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/db/IntegerSplitter.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/db/OracleDBRecordReader.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/db/OracleDataDrivenDBInputFormat.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/db/OracleDataDrivenDBRecordReader.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/db/TextSplitter.java 1212018 
>   /src/java/org/apache/sqoop/metastore/JobStorageFactory.java 1212018 
>   /src/java/org/apache/sqoop/metastore/hsqldb/AutoHsqldbStorage.java 1212018 
>   /src/java/org/apache/sqoop/metastore/hsqldb/HsqldbJobStorage.java 1212018 
>   /src/java/org/apache/sqoop/metastore/hsqldb/HsqldbMetaStore.java 1212018 
>   /src/java/org/apache/sqoop/tool/BaseSqoopTool.java 1212018 
>   /src/java/org/apache/sqoop/tool/CodeGenTool.java 1212018 
>   /src/java/org/apache/sqoop/tool/CreateHiveTableTool.java 1212018 
>   /src/java/org/apache/sqoop/tool/ImportAllTablesTool.java 1212018 
>   /src/java/org/apache/sqoop/tool/ImportTool.java 1212018 
>   /src/java/org/apache/sqoop/tool/JobTool.java 1212018 
>   /src/java/org/apache/sqoop/tool/MetastoreTool.java 1212018 
>   /src/java/org/apache/sqoop/tool/SqoopTool.java 1212018 
>   /src/java/org/apache/sqoop/util/AppendUtils.java 1212018 
>   /src/java/org/apache/sqoop/util/DirectImportUtils.java 1212018 
>   /src/java/org/apache/sqoop/util/TaskId.java 1212018 
>   /src/test/com/cloudera/sqoop/TestIncrementalImport.java 1212018 
>   /src/test/com/cloudera/sqoop/TestMerge.java 1212018 
>   /src/test/com/cloudera/sqoop/lib/TestBlobRef.java 1212018 
>   /src/test/com/cloudera/sqoop/lib/TestClobRef.java 1212018 
>   /src/test/com/cloudera/sqoop/lib/TestLargeObjectLoader.java 1212018 
>   /src/test/com/cloudera/sqoop/mapreduce/TestImportJob.java 1212018 
>   /src/test/com/cloudera/sqoop/testutil/ImportJobTestCase.java 1212018 
>   /src/test/com/cloudera/sqoop/testutil/InjectableConnManager.java 1212018 
>   /src/test/com/cloudera/sqoop/tool/TestToolPlugin.java 1212018 
> 
> Diff: https://reviews.apache.org/r/3126/diff
> 
> 
> Testing
> -------
> 
> "ant test" passes. I haven't had much success running sqoop at all, due to the bug this patch is trying to fix (SQOOP-384).
> 
> 
> Thanks,
> 
> Eric
> 
>


Re: Review Request: Added a ConfigurationHolder class, which adapts the older hadoop Configuration with the needs of sqoop.

Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3126/#review3829
-----------------------------------------------------------


Hi Eric,
thank you very much for your time and effort that you've spent on this issue. It looks like you did a lot of changes across entire sqoop code base.

The patch seems to me pretty heavy for just wrapping one method that is used only twice in sqoop code base. Instead of wrapping Configuration class into own object and then doing all the changes, I would propose to create our own method getInstances and put it somewhere (ConfigurationHelper seems as a good candidate) as a static method with following definition getInstances(Configuration conf, String name, Class<U> xface). This method would firstly test configuration object whether it defines method getInstances using JAVA reflection and if so, it would call the default implementation that is present in hadoop 0.21+ or CHD3. If not we would execute our own code that would simulate the default behavior for hadoop.

What do you think about that?

Jarcec

- Jarek


On 2011-12-09 21:41:48, Eric Wadsworth wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3126/
> -----------------------------------------------------------
> 
> (Updated 2011-12-09 21:41:48)
> 
> 
> Review request for Sqoop and Jarek Cecho.
> 
> 
> Summary
> -------
> 
> I added a ConfigurationHolder class, to try and make sqoop more compatible with hadoop 0.20.x.
> 
> 
> This addresses bug https://issues.apache.org/jira/browse/SQOOP-384.
>     https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/SQOOP-384
> 
> 
> Diffs
> -----
> 
>   /src/java/com/cloudera/sqoop/ConnFactory.java 1212018 
>   /src/java/com/cloudera/sqoop/Sqoop.java 1212018 
>   /src/java/com/cloudera/sqoop/SqoopOptions.java 1212018 
>   /src/java/com/cloudera/sqoop/config/ConfigurationHelper.java 1212018 
>   /src/java/com/cloudera/sqoop/hive/HiveImport.java 1212018 
>   /src/java/com/cloudera/sqoop/hive/TableDefWriter.java 1212018 
>   /src/java/com/cloudera/sqoop/io/CodecMap.java 1212018 
>   /src/java/com/cloudera/sqoop/io/LobFile.java 1212018 
>   /src/java/com/cloudera/sqoop/io/LobReaderCache.java 1212018 
>   /src/java/com/cloudera/sqoop/io/SplittingOutputStream.java 1212018 
>   /src/java/com/cloudera/sqoop/lib/LargeObjectLoader.java 1212018 
>   /src/java/com/cloudera/sqoop/manager/MySQLUtils.java 1212018 
>   /src/java/com/cloudera/sqoop/mapreduce/AvroJob.java 1212018 
>   /src/java/com/cloudera/sqoop/mapreduce/ExportJobBase.java 1212018 
>   /src/java/com/cloudera/sqoop/mapreduce/MySQLDumpMapper.java 1212018 
>   /src/java/com/cloudera/sqoop/mapreduce/db/DBConfiguration.java 1212018 
>   /src/java/com/cloudera/sqoop/mapreduce/db/DBRecordReader.java 1212018 
>   /src/java/com/cloudera/sqoop/mapreduce/db/DataDrivenDBInputFormat.java 1212018 
>   /src/java/com/cloudera/sqoop/mapreduce/db/DataDrivenDBRecordReader.java 1212018 
>   /src/java/com/cloudera/sqoop/mapreduce/db/OracleDBRecordReader.java 1212018 
>   /src/java/com/cloudera/sqoop/mapreduce/db/OracleDataDrivenDBRecordReader.java 1212018 
>   /src/java/com/cloudera/sqoop/metastore/JobStorageFactory.java 1212018 
>   /src/java/com/cloudera/sqoop/metastore/hsqldb/HsqldbMetaStore.java 1212018 
>   /src/java/com/cloudera/sqoop/util/DirectImportUtils.java 1212018 
>   /src/java/com/cloudera/sqoop/util/TaskId.java 1212018 
>   /src/java/org/apache/sqoop/ConnFactory.java 1212018 
>   /src/java/org/apache/sqoop/Sqoop.java 1212018 
>   /src/java/org/apache/sqoop/SqoopOptions.java 1212018 
>   /src/java/org/apache/sqoop/config/ConfigurationHelper.java 1212018 
>   /src/java/org/apache/sqoop/config/ConfigurationHolder.java PRE-CREATION 
>   /src/java/org/apache/sqoop/hbase/HBasePutProcessor.java 1212018 
>   /src/java/org/apache/sqoop/hive/HiveImport.java 1212018 
>   /src/java/org/apache/sqoop/hive/TableDefWriter.java 1212018 
>   /src/java/org/apache/sqoop/io/CodecMap.java 1212018 
>   /src/java/org/apache/sqoop/io/LobFile.java 1212018 
>   /src/java/org/apache/sqoop/io/LobReaderCache.java 1212018 
>   /src/java/org/apache/sqoop/io/SplittingOutputStream.java 1212018 
>   /src/java/org/apache/sqoop/lib/LargeObjectLoader.java 1212018 
>   /src/java/org/apache/sqoop/lib/LobRef.java 1212018 
>   /src/java/org/apache/sqoop/manager/DirectPostgresqlManager.java 1212018 
>   /src/java/org/apache/sqoop/manager/MySQLUtils.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/AsyncSqlRecordWriter.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/AutoProgressMapper.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/AvroExportMapper.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/AvroImportMapper.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/AvroJob.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/AvroOutputFormat.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/AvroRecordReader.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/CombineShimRecordReader.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/DataDrivenImportJob.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/DelegatingOutputFormat.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/ExportJobBase.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/ExportOutputFormat.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/HBaseImportJob.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/ImportJobBase.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/JobBase.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/MergeJob.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/MergeMapperBase.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/MergeRecord.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/MergeTextMapper.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/MySQLDumpImportJob.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/MySQLDumpMapper.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/MySQLExportJob.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/MySQLExportMapper.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/RawKeyTextOutputFormat.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/TextExportMapper.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/UpdateOutputFormat.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/db/BigDecimalSplitter.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/db/BooleanSplitter.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/db/DBConfiguration.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/db/DBInputFormat.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/db/DBRecordReader.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/db/DBSplitter.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/db/DataDrivenDBInputFormat.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/db/DataDrivenDBRecordReader.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/db/DateSplitter.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/db/FloatSplitter.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/db/IntegerSplitter.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/db/OracleDBRecordReader.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/db/OracleDataDrivenDBInputFormat.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/db/OracleDataDrivenDBRecordReader.java 1212018 
>   /src/java/org/apache/sqoop/mapreduce/db/TextSplitter.java 1212018 
>   /src/java/org/apache/sqoop/metastore/JobStorageFactory.java 1212018 
>   /src/java/org/apache/sqoop/metastore/hsqldb/AutoHsqldbStorage.java 1212018 
>   /src/java/org/apache/sqoop/metastore/hsqldb/HsqldbJobStorage.java 1212018 
>   /src/java/org/apache/sqoop/metastore/hsqldb/HsqldbMetaStore.java 1212018 
>   /src/java/org/apache/sqoop/tool/BaseSqoopTool.java 1212018 
>   /src/java/org/apache/sqoop/tool/CodeGenTool.java 1212018 
>   /src/java/org/apache/sqoop/tool/CreateHiveTableTool.java 1212018 
>   /src/java/org/apache/sqoop/tool/ImportAllTablesTool.java 1212018 
>   /src/java/org/apache/sqoop/tool/ImportTool.java 1212018 
>   /src/java/org/apache/sqoop/tool/JobTool.java 1212018 
>   /src/java/org/apache/sqoop/tool/MetastoreTool.java 1212018 
>   /src/java/org/apache/sqoop/tool/SqoopTool.java 1212018 
>   /src/java/org/apache/sqoop/util/AppendUtils.java 1212018 
>   /src/java/org/apache/sqoop/util/DirectImportUtils.java 1212018 
>   /src/java/org/apache/sqoop/util/TaskId.java 1212018 
>   /src/test/com/cloudera/sqoop/TestIncrementalImport.java 1212018 
>   /src/test/com/cloudera/sqoop/TestMerge.java 1212018 
>   /src/test/com/cloudera/sqoop/lib/TestBlobRef.java 1212018 
>   /src/test/com/cloudera/sqoop/lib/TestClobRef.java 1212018 
>   /src/test/com/cloudera/sqoop/lib/TestLargeObjectLoader.java 1212018 
>   /src/test/com/cloudera/sqoop/mapreduce/TestImportJob.java 1212018 
>   /src/test/com/cloudera/sqoop/testutil/ImportJobTestCase.java 1212018 
>   /src/test/com/cloudera/sqoop/testutil/InjectableConnManager.java 1212018 
>   /src/test/com/cloudera/sqoop/tool/TestToolPlugin.java 1212018 
> 
> Diff: https://reviews.apache.org/r/3126/diff
> 
> 
> Testing
> -------
> 
> "ant test" passes. I haven't had much success running sqoop at all, due to the bug this patch is trying to fix (SQOOP-384).
> 
> 
> Thanks,
> 
> Eric
> 
>