sqoop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Boglarka Egyed <b...@apache.org>
Subject Re: Review Request 67408: SQOOP-3326: Mainframe FTP listing for GDG should filter out non-GDG datasets in a heterogeneous listing
Date Thu, 04 Oct 2018 15:57:45 GMT

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


Fix it, then Ship it!




Hi Chris,

Thank you for your efforts on Mainframe support front!

Your change generally looks good to me and the unit, 3rd party tests passed with your patch.
I have one minor finding, could you please take a look?

Thanks,
Bogi


src/test/org/apache/sqoop/mapreduce/mainframe/TestMainframeFTPFileGdgEntryParser.java
Lines 54-73 (patched)
<https://reviews.apache.org/r/67408/#comment293544>

    "H19761 Tape" and "G0034V00" present in this code part more than once as raw strings.
This could become difficult to maintain in the future, I would suggest to use constants instead.


- Boglarka Egyed


On Sept. 21, 2018, 10:22 a.m., Chris Teoh wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/67408/
> -----------------------------------------------------------
> 
> (Updated Sept. 21, 2018, 10:22 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> -------
> 
> Mainframe FTP listing for GDG should filter out non-GDG datasets in a heterogeneous listing
as it grabs the last file and in the case where there are other datasets mixed in, the latest
file may not be the desired dataset.
> 
> 
> Diffs
> -----
> 
>   build.xml cd2e9e29 
>   src/java/org/apache/sqoop/mapreduce/mainframe/MainframeConfiguration.java 9d6a2fe7

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

>   src/java/org/apache/sqoop/util/MainframeFTPClientUtils.java 654721e3 
>   src/scripts/thirdpartytest/docker-compose/sqoop-thirdpartytest-db-services.yml 4648f545

>   src/test/org/apache/sqoop/manager/mainframe/MainframeManagerImportTest.java 3b8ed236

>   src/test/org/apache/sqoop/manager/mainframe/MainframeTestUtil.java 9f86f6cd 
>   src/test/org/apache/sqoop/mapreduce/mainframe/TestMainframeFTPFileGdgEntryParser.java
PRE-CREATION 
>   src/test/org/apache/sqoop/util/TestMainframeFTPClientUtils.java 90a85194 
> 
> 
> Diff: https://reviews.apache.org/r/67408/diff/3/
> 
> 
> Testing
> -------
> 
> Unit tests. Integration testing locally on developer machine.
> 
> 
> Thanks,
> 
> Chris Teoh
> 
>


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