> 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
> 
>

Reply via email to