sqoop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Venkat Ranganathan" <n....@live.com>
Subject Re: Review Request 22516: Support importing mainframe sequential datasets
Date Wed, 03 Sep 2014 05:04:11 GMT

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


I went through the code.  It looks good.  Thanks for taking the time to write this feature.
 I am not sure whether calling it MainframeImportTool is a proper name though.  We currently
support import from Z/OS DB2 also, so not sure whether this can confuse people.   Also, this
can be a specialization of a generic ftp record import method.   That said, I am OK with the
current names and formats if there are no further comments from others.


src/java/org/apache/sqoop/SqoopOptions.java
<https://reviews.apache.org/r/22516/#comment90872>

    Do we need to set tablename here - is it to overcome validations?



src/java/org/apache/sqoop/util/MainframeFTPClientUtils.java
<https://reviews.apache.org/r/22516/#comment90873>

    Why can't we throw the IOExceptin and a finally clause to wrap the entire body of code
to also disconnect from the FTP server.    That will be clean and will cover all exit cases



src/java/org/apache/sqoop/util/MainframeFTPClientUtils.java
<https://reviews.apache.org/r/22516/#comment90874>

    Same as above.



src/java/org/apache/sqoop/util/MainframeFTPClientUtils.java
<https://reviews.apache.org/r/22516/#comment90875>

    Throwing the exception here with the ioe as the cause would provide better diagnostics
in stack traces  (For example, 
       throw new IOException("....", ioe);


- Venkat Ranganathan


On Aug. 27, 2014, 7:55 p.m., Mariappan Asokan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22516/
> -----------------------------------------------------------
> 
> (Updated Aug. 27, 2014, 7:55 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> -------
> 
> This is to move mainframe datasets to Hadoop.
> 
> 
> Diffs
> -----
> 
>   ivy.xml 6335e01 
>   ivy/libraries.properties 6818b3e 
>   src/docs/man/common-args.txt e8d1f17 
>   src/docs/man/database-connection-args.txt PRE-CREATION 
>   src/docs/man/import-args.txt 2bb69ba 
>   src/docs/man/import-common-args.txt PRE-CREATION 
>   src/docs/man/mainframe-connection-args.txt PRE-CREATION 
>   src/docs/man/sqoop-import-mainframe.txt PRE-CREATION 
>   src/docs/man/sqoop-import.txt 00b1ec8 
>   src/docs/man/sqoop.txt febe827 
>   src/docs/user/SqoopUserGuide.txt 2e88887 
>   src/docs/user/basics.txt 7e5a76a 
>   src/docs/user/connecting-to-mainframe.txt PRE-CREATION 
>   src/docs/user/distributed-cache.txt PRE-CREATION 
>   src/docs/user/import-mainframe-purpose.txt PRE-CREATION 
>   src/docs/user/import-mainframe.txt PRE-CREATION 
>   src/docs/user/import.txt c5ffa50 
>   src/docs/user/intro.txt 99cd475 
>   src/docs/user/mainframe-common-args.txt PRE-CREATION 
>   src/docs/user/tools.txt 7d977d4 
>   src/docs/user/validation-args.txt 3cb5f66 
>   src/java/org/apache/sqoop/SqoopOptions.java 3ef5a97 
>   src/java/org/apache/sqoop/manager/MainframeManager.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/mainframe/MainframeConfiguration.java PRE-CREATION

>   src/java/org/apache/sqoop/mapreduce/mainframe/MainframeDatasetFTPRecordReader.java
PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/mainframe/MainframeDatasetImportMapper.java PRE-CREATION

>   src/java/org/apache/sqoop/mapreduce/mainframe/MainframeDatasetInputFormat.java PRE-CREATION

>   src/java/org/apache/sqoop/mapreduce/mainframe/MainframeDatasetInputSplit.java PRE-CREATION

>   src/java/org/apache/sqoop/mapreduce/mainframe/MainframeDatasetRecordReader.java PRE-CREATION

>   src/java/org/apache/sqoop/mapreduce/mainframe/MainframeImportJob.java PRE-CREATION

>   src/java/org/apache/sqoop/tool/MainframeImportTool.java PRE-CREATION 
>   src/java/org/apache/sqoop/tool/SqoopTool.java dbe429a 
>   src/java/org/apache/sqoop/util/MainframeFTPClientUtils.java PRE-CREATION 
>   src/test/org/apache/sqoop/manager/TestMainframeManager.java PRE-CREATION 
>   src/test/org/apache/sqoop/mapreduce/mainframe/TestMainframeDatasetFTPRecordReader.java
PRE-CREATION 
>   src/test/org/apache/sqoop/mapreduce/mainframe/TestMainframeDatasetInputFormat.java
PRE-CREATION 
>   src/test/org/apache/sqoop/mapreduce/mainframe/TestMainframeDatasetInputSplit.java PRE-CREATION

>   src/test/org/apache/sqoop/mapreduce/mainframe/TestMainframeImportJob.java PRE-CREATION

>   src/test/org/apache/sqoop/tool/TestMainframeImportTool.java PRE-CREATION 
>   src/test/org/apache/sqoop/util/TestMainframeFTPClientUtils.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/22516/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Mariappan Asokan
> 
>


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message