Home | About | Sematext search-lucene.com search-hadoop.com
 Search Hadoop and all its subprojects:

Switch to Threaded View
Sqoop, mail # dev - Review Request 13445: Fix for SQOOP-1167


Copy link to this message
-
Re: Review Request 13445: Fix for SQOOP-1167
Jarek Cecho 2013-09-03, 09:18

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13445/#review25846
-----------------------------------------------------------
Hi Venkat,
thank you very much for working on this JIRA! Couple of notes (mostly nits):
src/docs/user/hcatalog.txt
<https://reviews.apache.org/r/13445/#comment50400>

    Can we made a warning box here that not all direct connectors are supporting the HCatalog integration out of the box?

src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableImportMapper.java
<https://reviews.apache.org/r/13445/#comment50401>

    Since this class has been changed to abstract, would it make sense to define this method as abstract as well?

src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableTextImportMapper.java
<https://reviews.apache.org/r/13445/#comment50405>

    Nit: These variables seems to be unused.

src/test/org/apache/sqoop/hcat/HCatalogTestUtils.java
<https://reviews.apache.org/r/13445/#comment50402>

    The "IF EXISTS" clause seems to be failing on Netezza 6. I currently do not have access to version 7, but I'm wondering if this is new feature of version 7?
    
    Also it seems that couple of other classes have their own "DROP TABLE" statement definition. Maybe it would be worth cleaning that out and using only this util class to generate the drop table statement?

src/test/org/apache/sqoop/manager/netezza/DirectNetezzaHCatExportManualTest.java
<https://reviews.apache.org/r/13445/#comment50403>

    Super nit: s/testStaic/testStatic/

src/test/org/apache/sqoop/manager/netezza/DirectNetezzaHCatImportManualTest.java
<https://reviews.apache.org/r/13445/#comment50404>

    Super nit: s/testStaic/testStatic/

src/test/org/apache/sqoop/manager/netezza/NetezzaExportManualTest.java
<https://reviews.apache.org/r/13445/#comment50406>

    I'm wondering if this method "synonym" is necessary when it just call different protected method?

src/test/org/apache/sqoop/manager/netezza/NetezzaTestUtils.java
<https://reviews.apache.org/r/13445/#comment50407>

    I had difficulties to use system properties defined in this file to override connection credentials. I was trying to set clonevm of junit ant task to true, but that did not help me, so I've ended up tweaking the build system with following patch:
    
    diff --git a/build.xml b/build.xml
    index 39c6337..b56bc55 100644
    --- a/build.xml
    +++ b/build.xml
    @@ -322,6 +322,12 @@
       <property name="sqoop.test.db2.connectstring.username" value="SQOOP" />
       <property name="sqoop.test.db2.connectstring.password" value="SQOOP" />
    
    +  <property name="sqoop.test.netezza.host" value="nz-host" />
    +  <property name="sqoop.test.netezza.port" value="5480" />
    +  <property name="sqoop.test.netezza.username" value="ADMIN" />
    +  <property name="sqoop.test.netezza.password" value="password" />
    +  <property name="sqoop.test.netezza.db.name" value="SQOOP" />
    +  <property name="sqoop.test.netezza.table.name" value="EMPNZ" />
    
       <condition property="windows">
         <os family="windows" />
    @@ -901,6 +907,13 @@
           <sysproperty key="sqoop.test.db2.connectstring.username" value="${sqoop.test.db2.connectstring.username}" />
           <sysproperty key="sqoop.test.db2.connectstring.password" value="${sqoop.test.db2.connectstring.password}" />
    
    +      <sysproperty key="sqoop.test.netezza.host" value="${sqoop.test.netezza.host}" />
    +      <sysproperty key="sqoop.test.netezza.port" value="${sqoop.test.netezza.port}" />
    +      <sysproperty key="sqoop.test.netezza.username" value="${sqoop.test.netezza.username}" />
    +      <sysproperty key="sqoop.test.netezza.password" value="${sqoop.test.netezza.password}" />
    +      <sysproperty key="sqoop.test.netezza.db.name" value="${sqoop.test.netezza.db.name}" />
    +      <sysproperty key="sqoop.test.netezza.table.name" value="${sqoop.test.netezza.table.name}" />
    +
           <!-- Location of Hive logs -->
           <!--<sysproperty key="hive.log.dir"
                        value="${test.build.data}/sqoop/logs"/> -->
    
    Do you think that we should include something like this in the patch?
Jarcec

- Jarek Cecho
On Aug. 9, 2013, 6:11 p.m., Venkat Ranganathan wrote: