> On March 5, 2015, 10:01 p.m., Abraham Elmahrek wrote: > > Good stuff
Thanks for reviewing > On March 5, 2015, 10:01 p.m., Abraham Elmahrek wrote: > > src/java/org/apache/sqoop/manager/DirectNetezzaManager.java, lines 292-302 > > <https://reviews.apache.org/r/31749/diff/1/?file=885047#file885047line292> > > > > `conf.setBoolean(NETEZZA_CTRL_CHARS_OPT, > > cmdLine.hasOption(NETEZZA_CTRL_CHARS_LONG_ARG));` > > > > Same for next one as well. Thanks. I set it to be consistent with the rest of the codebase. But it does not hurt to make these changes. Will make the changes. > On March 5, 2015, 10:01 p.m., Abraham Elmahrek wrote: > > src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableExportMapper.java, > > lines 163-169 > > <https://reviews.apache.org/r/31749/diff/1/?file=885048#file885048line163> > > > > Seems we do this twice. Possible to pull out into a utility method? Sure. Will do. > On March 5, 2015, 10:01 p.m., Abraham Elmahrek wrote: > > src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableImportMapper.java, > > lines 221-226 > > <https://reviews.apache.org/r/31749/diff/1/?file=885049#file885049line221> > > > > See comment about utility class above Will do > On March 5, 2015, 10:01 p.m., Abraham Elmahrek wrote: > > src/java/org/apache/sqoop/util/FileUploader.java, lines 52-56 > > <https://reviews.apache.org/r/31749/diff/1/?file=885050#file885050line52> > > > > `if (!fs.exists(targetPath)) fs.mkdirs(targetPath)?` > > > > Or something similar to > > https://github.com/apache/hadoop/blob/branch-1.2/src/test/org/apache/hadoop/hdfs/TestDFSMkdirs.java#L51 > > or > > https://github.com/apache/hadoop/blob/branch-2.3/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/TestDFSMkdirs.java#L62 Good point. I was handling this part of the code differently before and did not clean up after changing. will do > On March 5, 2015, 10:01 p.m., Abraham Elmahrek wrote: > > src/java/org/apache/sqoop/util/FileUploader.java, line 63 > > <https://reviews.apache.org/r/31749/diff/1/?file=885050#file885050line63> > > > > Err confusing... did find this: > > http://stackoverflow.com/questions/12783968/copying-directory-from-local-system-to-hdfs-java-code. > > Maybe add a comment? Will add comments to say what the parameters mean. - Venkat ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/31749/#review75371 ----------------------------------------------------------- On March 6, 2015, 6:02 a.m., Venkat Ranganathan wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/31749/ > ----------------------------------------------------------- > > (Updated March 6, 2015, 6:02 a.m.) > > > Review request for Sqoop. > > > Repository: sqoop-trunk > > > Description > ------- > > The following options are enabled: ctrlchars, truncstring > > Also, now log-dir option is used to copy the generated logs from the Netezza > external table operation to the context filesystem so that they can be easily > perused > > Documentation has been updated > > > Diffs > ----- > > src/docs/user/connectors.txt fee40d9 > src/java/org/apache/sqoop/manager/DirectNetezzaManager.java 9ca8f63 > > src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableExportMapper.java > 3613ff2 > > src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableImportMapper.java > 2f4c152 > src/java/org/apache/sqoop/util/FileUploader.java PRE-CREATION > > src/test/org/apache/sqoop/manager/netezza/DirectNetezzaExportManualTest.java > f7b68ed > > Diff: https://reviews.apache.org/r/31749/diff/ > > > Testing > ------- > > Added tests to test for the new options. Ran all the Netezza tests with > this change and all passed. Ran manual tests on a cluster to make sure the > new options work > > > Thanks, > > Venkat Ranganathan > >
