> On Sept. 3, 2014, 5:04 a.m., Venkat Ranganathan wrote:
> > src/java/org/apache/sqoop/SqoopOptions.java, line 2176
> > <https://reviews.apache.org/r/22516/diff/2/?file=670466#file670466line2176>
> >
> >     Do we need to set tablename here - is it to overcome validations?

Yes, it is to make sure that nothing breaks.


> On Sept. 3, 2014, 5:04 a.m., Venkat Ranganathan wrote:
> > src/java/org/apache/sqoop/util/MainframeFTPClientUtils.java, line 131
> > <https://reviews.apache.org/r/22516/diff/2/?file=670477#file670477line131>
> >
> >     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

I totally agree with you.  I made the necessary code changes.


> On Sept. 3, 2014, 5:04 a.m., Venkat Ranganathan wrote:
> > src/java/org/apache/sqoop/util/MainframeFTPClientUtils.java, line 145
> > <https://reviews.apache.org/r/22516/diff/2/?file=670477#file670477line145>
> >
> >     Same as above.

Fixed.


> On Sept. 3, 2014, 5:04 a.m., Venkat Ranganathan wrote:
> > src/java/org/apache/sqoop/util/MainframeFTPClientUtils.java, line 154
> > <https://reviews.apache.org/r/22516/diff/2/?file=670477#file670477line154>
> >
> >     Throwing the exception here with the ioe as the cause would provide 
> > better diagnostics in stack traces  (For example, 
> >        throw new IOException("....", ioe);

Fixed.


- Mariappan


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


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

Reply via email to