sqoop-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jilani Shaik <jilani2...@gmail.com>
Subject Re: Review Request 57499: Sqoop incremental import - NULL column updates are not pulled into HBase table
Date Fri, 24 Mar 2017 06:12:01 GMT


> On March 18, 2017, 11:06 a.m., Boglarka Egyed wrote:
> > src/java/org/apache/sqoop/hbase/HBasePutProcessor.java
> > Line 131 (original), 134 (patched)
> > <https://reviews.apache.org/r/57499/diff/1/?file=1661134#file1661134line135>
> >
> >     Could you please use a more conventional naming to make the code more self-explanatory?
I think put should be changed to mutation from now as you are working with mutations instead
of puts only. This applies to your whole change in every file too.

updated in above patch(updated with unit test and based on below suggestions).


> On March 18, 2017, 11:06 a.m., Boglarka Egyed wrote:
> > src/java/org/apache/sqoop/hbase/HBasePutProcessor.java
> > Lines 136-141 (original), 138-154 (patched)
> > <https://reviews.apache.org/r/57499/diff/1/?file=1661134#file1661134line140>
> >
> >     Something with the indentation went wrong here, Sqoop uses 2 space indentation
by default, could you please correct it? Plus there are several trailing white spaces too
(marked as red rectangles in Diff).

updated in above patch(updated with unit test and based on below suggestions).


> On March 18, 2017, 11:06 a.m., Boglarka Egyed wrote:
> > src/java/org/apache/sqoop/hbase/ToStringPutTransformer.java
> > Lines 202 (patched)
> > <https://reviews.apache.org/r/57499/diff/1/?file=1661136#file1661136line206>
> >
> >     Please correct the indentation here too.

updated in above patch(updated with unit test and based on below suggestions).


> On March 18, 2017, 11:06 a.m., Boglarka Egyed wrote:
> > src/java/org/apache/sqoop/hbase/ToStringPutTransformer.java
> > Lines 210 (patched)
> > <https://reviews.apache.org/r/57499/diff/1/?file=1661136#file1661136line214>
> >
> >     Could you please avoid to comment old lines out and remove them instead?

updated in above patch(updated with unit test and based on below suggestions).


> On March 18, 2017, 11:06 a.m., Boglarka Egyed wrote:
> > src/java/org/apache/sqoop/mapreduce/HBaseBulkImportMapper.java
> > Line 84 (original), 86-88 (patched)
> > <https://reviews.apache.org/r/57499/diff/1/?file=1661137#file1661137line87>
> >
> >     Plese correct the indentation here too.

updated in above patch(updated with unit test and based on below suggestions).


- Jilani


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


On March 11, 2017, 6:16 a.m., Jilani Shaik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57499/
> -----------------------------------------------------------
> 
> (Updated March 11, 2017, 6:16 a.m.)
> 
> 
> Review request for Sqoop.
> 
> 
> Bugs: SQOOP-3149
>     https://issues.apache.org/jira/browse/SQOOP-3149
> 
> 
> Repository: sqoop-trunk
> 
> 
> Description
> -------
> 
> HBase delete is added as part of incremental data import, Modified the return type of
method which is responsible for holding list of Put objects to List of Mutation objects and
at the time of execution of insert or delete using the type of object in Mutation, executed
the actaul operation either insert or delete.
> 
> Jira ticket for the same: https://issues.apache.org/jira/browse/SQOOP-3149
> 
> Similar ticket to above:  SQOOP-3125
> 
> 
> Diffs
> -----
> 
>   src/java/org/apache/sqoop/hbase/HBasePutProcessor.java fdbe1276 
>   src/java/org/apache/sqoop/hbase/PutTransformer.java 533467e5 
>   src/java/org/apache/sqoop/hbase/ToStringPutTransformer.java 363e1456 
>   src/java/org/apache/sqoop/mapreduce/HBaseBulkImportMapper.java 58ccee7b 
> 
> 
> Diff: https://reviews.apache.org/r/57499/diff/1/
> 
> 
> Testing
> -------
> 
> Executed with jar prepared with these changes into hadoop cluster and tested bot import
and then incremental import.
> 
> 
> File Attachments
> ----------------
> 
> HBase delete support for incremental import
>   https://reviews.apache.org/media/uploaded/files/2017/03/11/5b1895fd-4c6b-42fa-8a92-4e093153e370__hbase_delete_support_in_incremental_import
> updated with unit test and based on below suggestions
>   https://reviews.apache.org/media/uploaded/files/2017/03/22/708f63ba-2d8a-4a47-ab67-c1d2776354fd__hbase_delete_support_in_incremental_unit_test_also
> 
> 
> Thanks,
> 
> Jilani Shaik
> 
>


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