sqoop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Szabolcs Vasas <vasas.szabo...@gmail.com>
Subject Re: Review Request 62492: SQOOP-3224: Mainframe FTP transfer should have an option to use binary mode for transfer
Date Fri, 01 Dec 2017 15:25:14 GMT

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




src/java/org/apache/sqoop/mapreduce/BinaryKeyOutputFormat.java
Lines 1 (patched)
<https://reviews.apache.org/r/62492/#comment270639>

    Can you please add license information to this new file?



src/java/org/apache/sqoop/mapreduce/BinaryKeyOutputFormat.java
Lines 2 (patched)
<https://reviews.apache.org/r/62492/#comment270646>

    It seems that big part of this class is a duplication of the content of RawKeyTextOutputFormat.
Can we eliminate the duplication?



src/java/org/apache/sqoop/mapreduce/DataDrivenImportJob.java
Line 86 (original), 89 (patched)
<https://reviews.apache.org/r/62492/#comment270645>

    Do you think we could introduce a new FileLayout value instead of transfer mode?
    Is the file layout used in mainframe import commands?
    If yes and we keep the mainframeFtpTransferMode option as well it could be confusing if
both of them are specified.



src/java/org/apache/sqoop/mapreduce/mainframe/MainframeDatasetBinaryImportMapper.java
Lines 1 (patched)
<https://reviews.apache.org/r/62492/#comment270640>

    Can you please add license information to this new file?



src/java/org/apache/sqoop/mapreduce/mainframe/MainframeDatasetBinaryImportMapper.java
Lines 2 (patched)
<https://reviews.apache.org/r/62492/#comment270647>

    Some parts of this class is a duplicate of code MainframeDatasetImportMapper, can you
please fix that?



src/java/org/apache/sqoop/mapreduce/mainframe/MainframeDatasetBinaryRecord.java
Lines 1 (patched)
<https://reviews.apache.org/r/62492/#comment270642>

    Can you please add license information to this new file?



src/java/org/apache/sqoop/mapreduce/mainframe/MainframeDatasetFTPRecordReader.java
Lines 75 (patched)
<https://reviews.apache.org/r/62492/#comment270648>

    This method is almost the exact duplicate of the previous initialize method, can you please
fix it?



src/test/org/apache/sqoop/mapreduce/mainframe/TestMainframeDatasetBinaryRecord.java
Lines 1 (patched)
<https://reviews.apache.org/r/62492/#comment270643>

    Can you please add license information to this new file?


- Szabolcs Vasas


On Nov. 29, 2017, 11:48 a.m., Chris Teoh wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/62492/
> -----------------------------------------------------------
> 
> (Updated Nov. 29, 2017, 11:48 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-3224
>     https://issues.apache.org/jira/browse/SQOOP-3224
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> -------
> 
> Added --transfermode {ascii|binary} to support FTP transfer mode switching.
> 
> 
> Diffs
> -----
> 
>   src/java/org/apache/sqoop/SqoopOptions.java 587d4e1d 
>   src/java/org/apache/sqoop/mapreduce/BinaryKeyOutputFormat.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/DataDrivenImportJob.java dc49282e 
>   src/java/org/apache/sqoop/mapreduce/mainframe/MainframeConfiguration.java ea54b07f

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

>   src/java/org/apache/sqoop/mapreduce/mainframe/MainframeDatasetFTPRecordReader.java
1f78384b 
>   src/java/org/apache/sqoop/mapreduce/mainframe/MainframeImportJob.java f222dc8f 
>   src/java/org/apache/sqoop/tool/MainframeImportTool.java 0cb91dbc 
>   src/java/org/apache/sqoop/util/MainframeFTPClientUtils.java 95bc0ecb 
>   src/test/org/apache/sqoop/mapreduce/mainframe/TestMainframeDatasetBinaryRecord.java
PRE-CREATION 
>   src/test/org/apache/sqoop/mapreduce/mainframe/TestMainframeDatasetFTPRecordReader.java
06141549 
>   src/test/org/apache/sqoop/tool/TestMainframeImportTool.java d51e33e5 
>   src/test/org/apache/sqoop/util/TestMainframeFTPClientUtils.java 90a85194 
> 
> 
> Diff: https://reviews.apache.org/r/62492/diff/7/
> 
> 
> Testing
> -------
> 
> Unit tests.
> 
> Functional testing on mainframe.
> 
> 
> Thanks,
> 
> Chris Teoh
> 
>


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