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