sqoop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Jarek Cecho" <jar...@apache.org>
Subject Re: Review Request 13445: Fix for SQOOP-1167
Date Tue, 03 Sep 2013 09:18:58 GMT

-----------------------------------------------------------
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:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13445/
> -----------------------------------------------------------
> 
> (Updated Aug. 9, 2013, 6:11 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-1167
>     https://issues.apache.org/jira/browse/SQOOP-1167
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> -------
> 
> This patch implements the following 
> 
> Enhance HCat support to allow direct mode connectors by
>    Creating helpers for export and import hcat mappers and refactor hcat mappers to use
that
>    Adding additional method for connection managers to declare their ability to exploit
this feature
>    Make the detection of compatibility between hcat and direct mode managers after connection
managers are created
>    As an example usecase, fix the netezza implementation to
>        Abstract the Netezza direct mode mappers and add hcat support
>        Fix Netezza connector implementation issues etc
>    Add documentation
>    Add Netezza tests to third party test suite
>    Move Netezza tests to org.apache.namespace to be consistent with requirements for
newly added tests
> 
> 
> Diffs
> -----
> 
>   src/docs/user/hcatalog.txt b8e495e 
>   src/java/org/apache/sqoop/manager/ConnManager.java f4b22f9 
>   src/java/org/apache/sqoop/manager/DirectNetezzaManager.java 4f36bf6 
>   src/java/org/apache/sqoop/mapreduce/ExportJobBase.java d0be570 
>   src/java/org/apache/sqoop/mapreduce/ImportJobBase.java ab7f21e 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableExportMapper.java
22b7af5 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableHCatExportMapper.java
PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableHCatImportMapper.java
PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableImportMapper.java
bcdc9e1 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaExternalTableTextImportMapper.java
PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/db/netezza/NetezzaJDBCStatementRunner.java 3a5df40

>   src/java/org/apache/sqoop/mapreduce/hcat/SqoopHCatExportHelper.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/hcat/SqoopHCatExportMapper.java 539cedf 
>   src/java/org/apache/sqoop/mapreduce/hcat/SqoopHCatImportHelper.java PRE-CREATION 
>   src/java/org/apache/sqoop/mapreduce/hcat/SqoopHCatImportMapper.java 4f0ff1b 
>   src/java/org/apache/sqoop/mapreduce/netezza/NetezzaExternalTableExportJob.java 7caf9be

>   src/java/org/apache/sqoop/mapreduce/netezza/NetezzaExternalTableImportJob.java 0f7c1b0

>   src/java/org/apache/sqoop/tool/BaseSqoopTool.java 0eca991 
>   src/test/com/cloudera/sqoop/ThirdPartyTests.java 0a080b6 
>   src/test/com/cloudera/sqoop/manager/DirectNetezzaExportManualTest.java aace924 
>   src/test/com/cloudera/sqoop/manager/NetezzaExportManualTest.java 43dd254 
>   src/test/com/cloudera/sqoop/manager/NetezzaImportManualTest.java 86f5bdd 
>   src/test/com/cloudera/sqoop/manager/NetezzaTestUtils.java 4bf05b8 
>   src/test/org/apache/sqoop/hcat/HCatalogExportTest.java 77bafcc 
>   src/test/org/apache/sqoop/hcat/HCatalogImportTest.java 293015e 
>   src/test/org/apache/sqoop/hcat/HCatalogTestUtils.java ddae5f5 
>   src/test/org/apache/sqoop/hcat/TestHCatalogBasic.java da803d0 
>   src/test/org/apache/sqoop/manager/netezza/DirectNetezzaExportManualTest.java PRE-CREATION

>   src/test/org/apache/sqoop/manager/netezza/DirectNetezzaHCatExportManualTest.java PRE-CREATION

>   src/test/org/apache/sqoop/manager/netezza/DirectNetezzaHCatImportManualTest.java PRE-CREATION

>   src/test/org/apache/sqoop/manager/netezza/NetezzaExportManualTest.java PRE-CREATION

>   src/test/org/apache/sqoop/manager/netezza/NetezzaImportManualTest.java PRE-CREATION

>   src/test/org/apache/sqoop/manager/netezza/NetezzaTestUtils.java PRE-CREATION 
> 
> Diff: https://reviews.apache.org/r/13445/diff/
> 
> 
> Testing
> -------
> 
> Add additional tests for testing this feature.   Ran all tests and all of them passed
> 
> 
> Thanks,
> 
> Venkat Ranganathan
> 
>


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