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

Switch to Plain View
Sqoop >> mail # dev >> Review Request: SQOOP-846 Provide Netezza connector


+
Venkat Ranganathan 2013-02-21, 16:49
+
Venkat Ranganathan 2013-02-22, 07:58
Copy link to this message
-
Re: Review Request: SQOOP-846 Provide Netezza connector

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9543/#review17010
-----------------------------------------------------------
Hi Venkat,
thank you very much for putting Netezza connector together. I greatly appreciate your time and effort. I've noticed that updated patch number 2 have only 12kb and is not the same as was uploaded into review board. Would you mind uploading the latest patch to the review board? It's much more easier for reviewer (well, it's much more easier for me).

Nevertheless, I've done high level review:

1) Would you mind removing trailing white space characters?

2) Would you mind adding appropriate section(s) into user guide?
src/java/org/apache/sqoop/manager/DirectNetezzaManager.java
<https://reviews.apache.org/r/9543/#comment36052>

    I felt that org.apache.sqoop.mapreduce packages is becoming a big mess, so I've started adding new connector-specific code into their own packages. For example:
    
    org.apache.sqoop.mapreduce.mysql
    org.apache.sqoop.mapreduce.sqlserver
    
    I would suggest to put the mapreduce files for Netezza into their own respective package:
    
    org.apache.sqoop.mapreduce.netezza
    
    What do you think?
    

src/java/org/apache/sqoop/manager/DirectNetezzaManager.java
<https://reviews.apache.org/r/9543/#comment36059>

    Tests are suggesting that staging table in Direct mode is not supported, though this class do not override supportsStagingForExport method that is returning true in parent class.

src/java/org/apache/sqoop/manager/DirectNetezzaManager.java
<https://reviews.apache.org/r/9543/#comment36053>

    Would you mind introducing finally block and closing the PreparedStatement and ResultSet? Something like
    
    } finally {
      if(rc != null) {
        rc.close();
      }
      if(ps != null) {
        ps.close();
      }
    }

src/java/org/apache/sqoop/manager/DirectNetezzaManager.java
<https://reviews.apache.org/r/9543/#comment36054>

    Nit: Would you mind putting the catch keyword on the same line as closing "}". The rest of code is following this code style, so it would be great to remain consistent.

src/java/org/apache/sqoop/manager/DirectNetezzaManager.java
<https://reviews.apache.org/r/9543/#comment36055>

    I can see that you're parsing extra arguments on several places (exportTable, importTable). What about moving this code into constructor?

src/java/org/apache/sqoop/manager/MySQLUtils.java
<https://reviews.apache.org/r/9543/#comment36056>

    Nit: Trailing white space characters.

src/java/org/apache/sqoop/manager/MySQLUtils.java
<https://reviews.apache.org/r/9543/#comment36057>

    Nit: Trailing white space characters.

src/test/com/cloudera/sqoop/manager/NetezzaTestUtils.java
<https://reviews.apache.org/r/9543/#comment36058>

    I'm not sure that you want to have your private IP publicly available in the source code.
Jarcec

- Jarek Cecho
On Feb. 22, 2013, 7:58 a.m., Venkat Ranganathan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9543/
> -----------------------------------------------------------
>
> (Updated Feb. 22, 2013, 7:58 a.m.)
>
>
> Review request for Sqoop and Jarek Cecho.
>
>
> Description
> -------
>
> This addresses SQOOP-846 (provide a Netezza connector)
>
>
> This addresses bug SQOOP-846.
>     https://issues.apache.org/jira/browse/SQOOP-846
>
>
> Diffs
> -----
>
>   src/java/org/apache/sqoop/manager/DirectNetezzaManager.java PRE-CREATION
>   src/java/org/apache/sqoop/manager/MySQLUtils.java ef18818
>   src/java/org/apache/sqoop/mapreduce/db/NetezzaExternalTableExportJob.java PRE-CREATION
>   src/java/org/apache/sqoop/mapreduce/db/NetezzaExternalTableExportMapper.java PRE-CREATION
>   src/java/org/apache/sqoop/mapreduce/db/NetezzaExternalTableImportMapper.java PRE-CREATION
+
Venkat Ranganathan 2013-02-25, 19:00
+
Venkat Ranganathan 2013-02-27, 19:54
+
Venkat Ranganathan 2013-02-25, 19:43
+
Venkat Ranganathan 2013-02-25, 19:46
+
Venkat Ranganathan 2013-02-25, 19:50
+
Venkat Ranganathan 2013-02-25, 20:53
+
Venkat Ranganathan 2013-02-25, 22:02
+
Jarek Cecho 2013-02-27, 01:01
+
Venkat Ranganathan 2013-02-28, 05:04
+
Venkat Ranganathan 2013-02-28, 05:10
+
Venkat Ranganathan 2013-02-27, 23:48
+
Venkat Ranganathan 2013-02-28, 01:53
+
Venkat Ranganathan 2013-02-28, 08:06
+
Jarek Cecho 2013-03-02, 19:26
+
Jarek Cecho 2013-03-02, 23:02
+
Venkat Ranganathan 2013-03-02, 22:52
+
Venkat Ranganathan 2013-03-05, 01:26
+
Venkat Ranganathan 2013-03-03, 05:16