You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sqoop.apache.org by Venkat Ranganathan <n....@live.com> on 2015/03/05 01:13:39 UTC
Review Request 31749: SQOOP-2164: Enhance the Netezza Connector for
Sqoop to support new options and copy logs to HDFS
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31749/
-----------------------------------------------------------
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
Re: Review Request 31749: SQOOP-2164: Enhance the Netezza Connector
for Sqoop to support new options and copy logs to HDFS
Posted by Abraham Elmahrek <ab...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31749/#review76044
-----------------------------------------------------------
Ship it!
Ship It!
- Abraham Elmahrek
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
>
>
Re: Review Request 31749: SQOOP-2164: Enhance the Netezza Connector
for Sqoop to support new options and copy logs to HDFS
Posted by Venkat Ranganathan <n....@live.com>.
-----------------------------------------------------------
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.
Changes
-------
Incorporated review comments
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 (updated)
-----
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
Re: Review Request 31749: SQOOP-2164: Enhance the Netezza Connector
for Sqoop to support new options and copy logs to HDFS
Posted by Venkat Ranganathan <n....@live.com>.
> 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
>
>
Re: Review Request 31749: SQOOP-2164: Enhance the Netezza Connector
for Sqoop to support new options and copy logs to HDFS
Posted by Abraham Elmahrek <ab...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/31749/#review75371
-----------------------------------------------------------
Good stuff
src/java/org/apache/sqoop/manager/DirectNetezzaManager.java
<https://reviews.apache.org/r/31749/#comment122374>
`conf.setBoolean(NETEZZA_CTRL_CHARS_OPT, cmdLine.hasOption(NETEZZA_CTRL_CHARS_LONG_ARG));`
Same for next one as well.
src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableExportMapper.java
<https://reviews.apache.org/r/31749/#comment122426>
Seems we do this twice. Possible to pull out into a utility method?
src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableImportMapper.java
<https://reviews.apache.org/r/31749/#comment122427>
See comment about utility class above
src/java/org/apache/sqoop/util/FileUploader.java
<https://reviews.apache.org/r/31749/#comment122417>
`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
src/java/org/apache/sqoop/util/FileUploader.java
<https://reviews.apache.org/r/31749/#comment122424>
Err confusing... did find this: http://stackoverflow.com/questions/12783968/copying-directory-from-local-system-to-hdfs-java-code. Maybe add a comment?
- Abraham Elmahrek
On March 5, 2015, 12:13 a.m., Venkat Ranganathan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/31749/
> -----------------------------------------------------------
>
> (Updated March 5, 2015, 12:13 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
>
>