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


Overall looks great. Please address the comments below and I am glag there is a 
separate ticket for the integration tests to use Hbase as a TO connector.


connector/connector-hbase/src/main/java/org/apache/sqoop/connector/hbase/HBaseConnector.java
<https://reviews.apache.org/r/26581/#comment107965>

    Please add a few comments / java doc, will be educative for someone reading 
this code.



connector/connector-hbase/src/main/java/org/apache/sqoop/connector/hbase/HBaseExecutor.java
<https://reviews.apache.org/r/26581/#comment107966>

    I had this same question in KiteConnector as well , do we need to 
createConnection every time?



connector/connector-hbase/src/main/java/org/apache/sqoop/connector/hbase/HBaseLoader.java
<https://reviews.apache.org/r/26581/#comment107972>

    is it ok for the loader to write JODA date time objects?
    
    Can we have more unit tests per data type on how it gets written ?
    
    Or is this something we will cover in integration tests?



connector/connector-hbase/src/main/java/org/apache/sqoop/connector/hbase/HBaseToInitializer.java
<https://reviews.apache.org/r/26581/#comment107967>

    Please remove these TODOs. if you want to fix this create a ticket, else 
add a NOTE describing the limitation



connector/connector-hbase/src/main/java/org/apache/sqoop/connector/hbase/HBaseToInitializer.java
<https://reviews.apache.org/r/26581/#comment107968>

    There is a JarUtil in the connector-sdk, might be good to see if that 
helps, or move this code to the util so that other conenctors can use, every 
connector is hacking how the dependend jars are laoded, as Qian says



connector/connector-hbase/src/main/java/org/apache/sqoop/connector/hbase/HBaseToInitializer.java
<https://reviews.apache.org/r/26581/#comment107969>

    just use the NullSchema, if there is no schema, you can drop overriding 
this method completelt.



connector/connector-hbase/src/main/java/org/apache/sqoop/connector/hbase/configuration/LinkConfig.java
<https://reviews.apache.org/r/26581/#comment107970>

    are there no validations for this field?



connector/connector-hbase/src/main/java/org/apache/sqoop/connector/hbase/configuration/ToJobConfig.java
<https://reviews.apache.org/r/26581/#comment107971>

    ditto: does not any of these have any validations? Ots ok if you want to 
address in a new ticket, please add that in


- Veena Basavaraj


On Dec. 13, 2014, 4:26 p.m., Abraham Elmahrek wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/26581/
> -----------------------------------------------------------
> 
> (Updated Dec. 13, 2014, 4:26 p.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-1156
>     https://issues.apache.org/jira/browse/SQOOP-1156
> 
> 
> Repository: sqoop-sqoop2
> 
> 
> Description
> -------
> 
> commit b9af5c1556e2c88a3c861950a6a296733fa1e4e7
> Author: Abraham Elmahrek <[email protected]>
> Date:   Thu Oct 9 22:56:34 2014 -0700
> 
>     SQOOP-1156: HBase connector
> 
> :000000 100644 0000000... 61dd408... A  connector/connector-hbase/pom.xml
> :000000 100644 0000000... b0e4ea0... A  
> connector/connector-hbase/src/main/java/org/apache/sqoop/connector/hbase/HBaseConnector.java
> :000000 100644 0000000... 975cb40... A  
> connector/connector-hbase/src/main/java/org/apache/sqoop/connector/hbase/HBaseConnectorConstants.java
> :000000 100644 0000000... 2e08bfd... A  
> connector/connector-hbase/src/main/java/org/apache/sqoop/connector/hbase/HBaseConnectorError.java
> :000000 100644 0000000... cf632c9... A  
> connector/connector-hbase/src/main/java/org/apache/sqoop/connector/hbase/HBaseConnectorRepositoryUpgrader.java
> :000000 100644 0000000... bc61993... A  
> connector/connector-hbase/src/main/java/org/apache/sqoop/connector/hbase/HBaseExecutor.java
> :000000 100644 0000000... 991846f... A  
> connector/connector-hbase/src/main/java/org/apache/sqoop/connector/hbase/HBaseLoader.java
> :000000 100644 0000000... 325591c... A  
> connector/connector-hbase/src/main/java/org/apache/sqoop/connector/hbase/HBaseToDestroyer.java
> :000000 100644 0000000... 51fd885... A  
> connector/connector-hbase/src/main/java/org/apache/sqoop/connector/hbase/HBaseToInitializer.java
> :000000 100644 0000000... 6e306b1... A  
> connector/connector-hbase/src/main/java/org/apache/sqoop/connector/hbase/configuration/LinkConfig.java
> :000000 100644 0000000... a4b825c... A  
> connector/connector-hbase/src/main/java/org/apache/sqoop/connector/hbase/configuration/LinkConfiguration.java
> :000000 100644 0000000... 15feb54... A  
> connector/connector-hbase/src/main/java/org/apache/sqoop/connector/hbase/configuration/ToJobConfig.java
> :000000 100644 0000000... cc71b78... A  
> connector/connector-hbase/src/main/java/org/apache/sqoop/connector/hbase/configuration/ToJobConfiguration.java
> :000000 100644 0000000... 15a9425... A  
> connector/connector-hbase/src/main/resources/hbase-connector-resources.properties
> :000000 100644 0000000... 1fc360e... A  
> connector/connector-hbase/src/main/resources/sqoopconnector.properties
> :000000 100644 0000000... c78042a... A  
> connector/connector-hbase/src/test/java/org/apache/sqoop/connector/hbase/TestExecutor.java
> :000000 100644 0000000... 60f8217... A  
> connector/connector-hbase/src/test/java/org/apache/sqoop/connector/hbase/TestLoader.java
> :000000 100644 0000000... 44ffced... A  
> connector/connector-hbase/src/test/resources/log4j.properties
> :100644 100644 e98a0fc... 35c665e... M  connector/pom.xml
> :100644 100644 f25a29f... a556bcf... M  pom.xml
> :100644 100644 67baaa5... 21a1fa9... M  server/pom.xml
> :100644 100644 7a80710... fbd4e84... M  test/pom.xml
> 
> 
> Diffs
> -----
> 
>   connector/connector-hbase/pom.xml PRE-CREATION 
>   
> connector/connector-hbase/src/main/java/org/apache/sqoop/connector/hbase/HBaseConnector.java
>  PRE-CREATION 
>   
> connector/connector-hbase/src/main/java/org/apache/sqoop/connector/hbase/HBaseConnectorConstants.java
>  PRE-CREATION 
>   
> connector/connector-hbase/src/main/java/org/apache/sqoop/connector/hbase/HBaseConnectorError.java
>  PRE-CREATION 
>   
> connector/connector-hbase/src/main/java/org/apache/sqoop/connector/hbase/HBaseConnectorUpgrader.java
>  PRE-CREATION 
>   
> connector/connector-hbase/src/main/java/org/apache/sqoop/connector/hbase/HBaseExecutor.java
>  PRE-CREATION 
>   
> connector/connector-hbase/src/main/java/org/apache/sqoop/connector/hbase/HBaseLoader.java
>  PRE-CREATION 
>   
> connector/connector-hbase/src/main/java/org/apache/sqoop/connector/hbase/HBaseToDestroyer.java
>  PRE-CREATION 
>   
> connector/connector-hbase/src/main/java/org/apache/sqoop/connector/hbase/HBaseToInitializer.java
>  PRE-CREATION 
>   
> connector/connector-hbase/src/main/java/org/apache/sqoop/connector/hbase/configuration/LinkConfig.java
>  PRE-CREATION 
>   
> connector/connector-hbase/src/main/java/org/apache/sqoop/connector/hbase/configuration/LinkConfiguration.java
>  PRE-CREATION 
>   
> connector/connector-hbase/src/main/java/org/apache/sqoop/connector/hbase/configuration/ToJobConfig.java
>  PRE-CREATION 
>   
> connector/connector-hbase/src/main/java/org/apache/sqoop/connector/hbase/configuration/ToJobConfiguration.java
>  PRE-CREATION 
>   
> connector/connector-hbase/src/main/resources/generic-jdbc-connector-config.properties
>  PRE-CREATION 
>   connector/connector-hbase/src/main/resources/sqoopconnector.properties 
> PRE-CREATION 
>   
> connector/connector-hbase/src/test/java/org/apache/sqoop/connector/hbase/TestHBaseExecutor.java
>  PRE-CREATION 
>   
> connector/connector-hbase/src/test/java/org/apache/sqoop/connector/hbase/TestHBaseLoader.java
>  PRE-CREATION 
>   connector/connector-hbase/src/test/resources/log4j.properties PRE-CREATION 
>   connector/pom.xml dfa7e88 
>   pom.xml efb9659 
>   server/pom.xml 77477ee 
>   test/pom.xml 35d36c1 
> 
> Diff: https://reviews.apache.org/r/26581/diff/
> 
> 
> Testing
> -------
> 
> mvn clean verify + can transfer from mysql to hbase.
> 
> 
> Thanks,
> 
> Abraham Elmahrek
> 
>

Reply via email to