sqoop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Veena Basavaraj" <vbasava...@cloudera.com>
Subject Re: Review Request 28614: SQOOP-1678: Sqoop2: Configurable null values
Date Wed, 10 Dec 2014 17:57:16 GMT

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28614/#review64573
-----------------------------------------------------------


Nice enhancement, have a few comments,


connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsExtractor.java
<https://reviews.apache.org/r/28614/#comment107339>

    nitpick, why not pass thse into the fucntions, less state in the classes is good pattern
to avoid any future multi thread issue



connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsUtils.java
<https://reviews.apache.org/r/28614/#comment107340>

    modifiable probably use the ssame terminilogy as transform to keep it consistent



connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsUtils.java
<https://reviews.apache.org/r/28614/#comment107341>

    probably at some point this can be an interface optinally implemented by conenctors. Since
even JDBC needs to do this, Right now this code was added in extractor in 1830 and needs to
bed added in loader as well, since we found a bug



connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/FromJobConfig.java
<https://reviews.apache.org/r/28614/#comment107342>

    please add more help text in the .properites



connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestLoader.java
<https://reviews.apache.org/r/28614/#comment107343>

    nice tests.!


- Veena Basavaraj


On Dec. 2, 2014, 3:31 p.m., Abraham Elmahrek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28614/
> -----------------------------------------------------------
> 
> (Updated Dec. 2, 2014, 3:31 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-1678
>     https://issues.apache.org/jira/browse/SQOOP-1678
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> commit 3898ce500f7aac0104de05e3c2c33d1c6cf7d13c
> Author: Abraham Elmahrek <abraham@elmahrek.com>
> Date:   Tue Dec 2 15:23:12 2014 -0800
> 
>     SQOOP-1678: Sqoop2: Configurable null values
> 
> :100644 100644 6e369c6... 788dfd2... M  connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsConstants.java
> :100644 100644 2586f94... d92e296... M  connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsExtractor.java
> :100644 100644 6c57cf2... e7b1302... M  connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsLoader.java
> :100644 100644 352ee17... f3ac167... M  connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsUtils.java
> :100644 100644 509d772... 89ff9aa... M  connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/FromJobConfig.java
> :100644 100644 abddbfb... b7a9c3d... M  connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/ToJobConfig.java
> :100644 100644 0a6369f... 52846ed... M  connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestExtractor.java
> :100644 100644 6eae7fd... ac44595... M  connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestHdfsBase.java
> :100644 100644 63e14ae... 15e3c14... M  connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestHdfsUtils.java
> :100644 100644 b404c34... be57fa0... M  connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestLoader.java
> 
> 
> Diffs
> -----
> 
>   connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsConstants.java
6e369c6 
>   connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsExtractor.java
2586f94 
>   connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsLoader.java
6c57cf2 
>   connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/HdfsUtils.java
352ee17 
>   connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/FromJobConfig.java
509d772 
>   connector/connector-hdfs/src/main/java/org/apache/sqoop/connector/hdfs/configuration/ToJobConfig.java
abddbfb 
>   connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestExtractor.java
0a6369f 
>   connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestHdfsBase.java
6eae7fd 
>   connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestHdfsUtils.java
63e14ae 
>   connector/connector-hdfs/src/test/java/org/apache/sqoop/connector/hdfs/TestLoader.java
b404c34 
> 
> Diff: https://reviews.apache.org/r/28614/diff/
> 
> 
> Testing
> -------
> 
> 'mvn verify' with local runner.
> 
> 
> Thanks,
> 
> Abraham Elmahrek
> 
>


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