You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sqoop.apache.org by Zhancheng Deng <zh...@163.com> on 2013/07/30 07:32:46 UTC
Re: Review Request 13052: Add the --bulk-load-dir option to support the
HBase doBulkLoad function
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13052/
-----------------------------------------------------------
(Updated July 30, 2013, 5:32 a.m.)
Review request for Sqoop, Jarek Cecho and vasanthkumar.
Changes
-------
src/java/com/cloudera/sqoop/hbase/HBasePutProcessor.java (425b0f4)
src/java/org/apache/sqoop/SqoopOptions.java (01805f9)
src/java/org/apache/sqoop/hbase/HBasePutProcessor.java (9ceb5bd)
src/java/org/apache/sqoop/hbase/ToStringPutTransformer.java (5ccf311)
src/java/org/apache/sqoop/manager/SqlManager.java (2a4992d)
src/java/org/apache/sqoop/mapreduce/HBaseBulkImportJob.java (PRE-CREATION)
src/java/org/apache/sqoop/mapreduce/HBaseBulkImportMapper.java (PRE-CREATION)
src/java/org/apache/sqoop/tool/BaseSqoopTool.java (0eca991)
Summary (updated)
-----------------
Add the --bulk-load-dir option to support the HBase doBulkLoad function
Repository: sqoop-trunk
Description
-------
SQOOP-1032: Add the --bulk-load-dir option to support the HBase doBulkLoad function
Diffs
-----
src/java/com/cloudera/sqoop/hbase/HBasePutProcessor.java 425b0f4
src/java/org/apache/sqoop/SqoopOptions.java 01805f9
src/java/org/apache/sqoop/hbase/HBasePutProcessor.java 9ceb5bd
src/java/org/apache/sqoop/hbase/ToStringPutTransformer.java 5ccf311
src/java/org/apache/sqoop/manager/SqlManager.java 2a4992d
src/java/org/apache/sqoop/mapreduce/HBaseBulkImportJob.java PRE-CREATION
src/java/org/apache/sqoop/mapreduce/HBaseBulkImportMapper.java PRE-CREATION
src/java/org/apache/sqoop/tool/BaseSqoopTool.java 0eca991
Diff: https://reviews.apache.org/r/13052/diff/
Testing
-------
Thanks,
Zhancheng Deng
Re: Review Request 13052: Add the --bulk-load-dir option to support the
HBase doBulkLoad function
Posted by Alexandre Normand <al...@gmail.com>.
> On Sept. 2, 2013, 2:30 p.m., Jarek Cecho wrote:
> > Hi Zhancheng,
> > thank you for working on this JIRA, appreciated! Would you mind adding test cases to ensure that this functionality works as expected?
If I'm not mistaken, integration testing bulk loading is still very hard/impossible because part of the configureIncrementalLoad needs to read the partition list which fails with the combination of a mini cluster + LocalJobRunner. I'm not advocating for no tests but maybe the compromise here would be to extract the bulk loading operations and test the rest of the logic of the HBaseBulkImportJob.
- Alexandre
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13052/#review25824
-----------------------------------------------------------
On July 30, 2013, 5:32 a.m., Zhancheng Deng wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13052/
> -----------------------------------------------------------
>
> (Updated July 30, 2013, 5:32 a.m.)
>
>
> Review request for Sqoop, Jarek Cecho and vasanthkumar.
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> SQOOP-1032: Add the --bulk-load-dir option to support the HBase doBulkLoad function
>
>
> Diffs
> -----
>
> src/java/com/cloudera/sqoop/hbase/HBasePutProcessor.java 425b0f4
> src/java/org/apache/sqoop/SqoopOptions.java 01805f9
> src/java/org/apache/sqoop/hbase/HBasePutProcessor.java 9ceb5bd
> src/java/org/apache/sqoop/hbase/ToStringPutTransformer.java 5ccf311
> src/java/org/apache/sqoop/manager/SqlManager.java 2a4992d
> src/java/org/apache/sqoop/mapreduce/HBaseBulkImportJob.java PRE-CREATION
> src/java/org/apache/sqoop/mapreduce/HBaseBulkImportMapper.java PRE-CREATION
> src/java/org/apache/sqoop/tool/BaseSqoopTool.java 0eca991
>
> Diff: https://reviews.apache.org/r/13052/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Zhancheng Deng
>
>
Re: Review Request 13052: Add the --bulk-load-dir option to support the
HBase doBulkLoad function
Posted by Jarek Cecho <ja...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/13052/#review25824
-----------------------------------------------------------
Hi Zhancheng,
thank you for working on this JIRA, appreciated! Would you mind adding test cases to ensure that this functionality works as expected?
src/java/com/cloudera/sqoop/hbase/HBasePutProcessor.java
<https://reviews.apache.org/r/13052/#comment50370>
Let's not alter the classes in com.cloudera package. They are already deprecated and should not be used.
src/java/org/apache/sqoop/SqoopOptions.java
<https://reviews.apache.org/r/13052/#comment50371>
Nit: Trailing white space.
src/java/org/apache/sqoop/SqoopOptions.java
<https://reviews.apache.org/r/13052/#comment50372>
Nit: Trailing white space.
src/java/org/apache/sqoop/SqoopOptions.java
<https://reviews.apache.org/r/13052/#comment50374>
The comment seems to be off.
src/java/org/apache/sqoop/SqoopOptions.java
<https://reviews.apache.org/r/13052/#comment50373>
Nit: Trailing white space.
src/java/org/apache/sqoop/hbase/HBasePutProcessor.java
<https://reviews.apache.org/r/13052/#comment50375>
Nit: Trailing white space.
src/java/org/apache/sqoop/manager/SqlManager.java
<https://reviews.apache.org/r/13052/#comment50376>
Nit: Trailing white space.
src/java/org/apache/sqoop/manager/SqlManager.java
<https://reviews.apache.org/r/13052/#comment50377>
Nit: Trailing white space.
src/java/org/apache/sqoop/mapreduce/HBaseBulkImportJob.java
<https://reviews.apache.org/r/13052/#comment50378>
User had to specify the bulk load directory at this point, so it would be better to rather die quickly here.
src/java/org/apache/sqoop/mapreduce/HBaseBulkImportJob.java
<https://reviews.apache.org/r/13052/#comment50380>
User had to specify the bulk load directory at this point, so it would be better to rather die quickly than run the slower non bulk import.
src/java/org/apache/sqoop/mapreduce/HBaseBulkImportJob.java
<https://reviews.apache.org/r/13052/#comment50379>
Nit: Trailing white space.
src/java/org/apache/sqoop/mapreduce/HBaseBulkImportMapper.java
<https://reviews.apache.org/r/13052/#comment50381>
Nit: Trailing white space.
src/java/org/apache/sqoop/tool/BaseSqoopTool.java
<https://reviews.apache.org/r/13052/#comment50382>
Nit: Trailing white space.
Jarcec
- Jarek Cecho
On July 30, 2013, 5:32 a.m., Zhancheng Deng wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/13052/
> -----------------------------------------------------------
>
> (Updated July 30, 2013, 5:32 a.m.)
>
>
> Review request for Sqoop, Jarek Cecho and vasanthkumar.
>
>
> Repository: sqoop-trunk
>
>
> Description
> -------
>
> SQOOP-1032: Add the --bulk-load-dir option to support the HBase doBulkLoad function
>
>
> Diffs
> -----
>
> src/java/com/cloudera/sqoop/hbase/HBasePutProcessor.java 425b0f4
> src/java/org/apache/sqoop/SqoopOptions.java 01805f9
> src/java/org/apache/sqoop/hbase/HBasePutProcessor.java 9ceb5bd
> src/java/org/apache/sqoop/hbase/ToStringPutTransformer.java 5ccf311
> src/java/org/apache/sqoop/manager/SqlManager.java 2a4992d
> src/java/org/apache/sqoop/mapreduce/HBaseBulkImportJob.java PRE-CREATION
> src/java/org/apache/sqoop/mapreduce/HBaseBulkImportMapper.java PRE-CREATION
> src/java/org/apache/sqoop/tool/BaseSqoopTool.java 0eca991
>
> Diff: https://reviews.apache.org/r/13052/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Zhancheng Deng
>
>