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

Review request for Sqoop.
Description
-------

Problem is that we're creating Table descriptor locally, which is not importing remote table families automatically. I've enhanced the code to retrieve remote table descriptor prior checking if the table family is present.
This addresses bug SQOOP-600.
    https://issues.apache.org/jira/browse/SQOOP-600
Diffs
-----

  src/java/org/apache/sqoop/mapreduce/HBaseImportJob.java 6a784d2

Diff: https://reviews.apache.org/r/7014/diff/
Testing
-------

I've done only manual testing on HBase 0.94. Automatic tests are disabled in different profiles other than hadoopversion={20,100} and I'll work on getting them working later.
Thanks,

Jarek Cecho

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

Ship it!
I only have a minor comment. Please see inline.
src/java/org/apache/sqoop/mapreduce/HBaseImportJob.java
<https://reviews.apache.org/r/7014/#comment24412>

    Can you move this line to inside the if statement? So it will look like:
    
    HTableDescriptor tableDesc = null;
    ...
    if (admin.tableExists(tableName)) {
       tableDesc = new HTableDescriptor(tableName);
       ...
    } else {
       tableDesc = admin.getTableDescriptor(Bytes.toBytes(tableName));
       ...
    }
    
    The reasons are:
    1) This seems cleaner to me.
    2) Currently, the instantiation of HTableDescritor is wasted when the table already exists. That is, we instantiate a new HTableDescriptor and overwrite the reference with another.
- Cheolsoo Park
On Sept. 11, 2012, 11:03 a.m., Jarek Cecho wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7014/
> -----------------------------------------------------------
>
> (Updated Sept. 11, 2012, 11:03 a.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> Problem is that we're creating Table descriptor locally, which is not importing remote table families automatically. I've enhanced the code to retrieve remote table descriptor prior checking if the table family is present.
>
>
> This addresses bug SQOOP-600.
>     https://issues.apache.org/jira/browse/SQOOP-600
>
>
> Diffs
> -----
>
>   src/java/org/apache/sqoop/mapreduce/HBaseImportJob.java 6a784d2
>
> Diff: https://reviews.apache.org/r/7014/diff/
>
>
> Testing
> -------
>
> I've done only manual testing on HBase 0.94. Automatic tests are disabled in different profiles other than hadoopversion={20,100} and I'll work on getting them working later.
>
>
> Thanks,
>
> Jarek Cecho
>
>

> On Sept. 11, 2012, 11:49 p.m., Cheolsoo Park wrote:
> > I only have a minor comment. Please see inline.

Thank you for your review Cheolsoo, I appreciate your time.
> On Sept. 11, 2012, 11:49 p.m., Cheolsoo Park wrote:
> > src/java/org/apache/sqoop/mapreduce/HBaseImportJob.java, line 142
> > <https://reviews.apache.org/r/7014/diff/1/?file=152587#file152587line142>
> >
> >     Can you move this line to inside the if statement? So it will look like:
> >    
> >     HTableDescriptor tableDesc = null;
> >     ...
> >     if (admin.tableExists(tableName)) {
> >        tableDesc = new HTableDescriptor(tableName);
> >        ...
> >     } else {
> >        tableDesc = admin.getTableDescriptor(Bytes.toBytes(tableName));
> >        ...
> >     }
> >    
> >     The reasons are:
> >     1) This seems cleaner to me.
> >     2) Currently, the instantiation of HTableDescritor is wasted when the table already exists. That is, we instantiate a new HTableDescriptor and overwrite the reference with another.

Make sense to me, I'll update the patch.
- Jarek
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7014/#review11361
-----------------------------------------------------------
On Sept. 11, 2012, 11:03 a.m., Jarek Cecho wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7014/
> -----------------------------------------------------------
>
> (Updated Sept. 11, 2012, 11:03 a.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> Problem is that we're creating Table descriptor locally, which is not importing remote table families automatically. I've enhanced the code to retrieve remote table descriptor prior checking if the table family is present.
>
>
> This addresses bug SQOOP-600.
>     https://issues.apache.org/jira/browse/SQOOP-600
>
>
> Diffs
> -----
>
>   src/java/org/apache/sqoop/mapreduce/HBaseImportJob.java 6a784d2
>
> Diff: https://reviews.apache.org/r/7014/diff/
>
>
> Testing
> -------
>
> I've done only manual testing on HBase 0.94. Automatic tests are disabled in different profiles other than hadoopversion={20,100} and I'll work on getting them working later.
>
>
> Thanks,
>
> Jarek Cecho
>
>

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7014/#review11429
-----------------------------------------------------------
ant test (100): Success. I think the Hbase tests are disabled for hadoopversion 100 as well. Let me know.
ant test (20): Hbase tests timed out for me(TestHbaseImport and TestHbaseQueryImport). These don't seem to be a regression as they are failing on my m/c for current trunk as well. If they work for you, I have a ship it.

- Abhijeet Gaikwad
On Sept. 11, 2012, 11:03 a.m., Jarek Cecho wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7014/
> -----------------------------------------------------------
>
> (Updated Sept. 11, 2012, 11:03 a.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> Problem is that we're creating Table descriptor locally, which is not importing remote table families automatically. I've enhanced the code to retrieve remote table descriptor prior checking if the table family is present.
>
>
> This addresses bug SQOOP-600.
>     https://issues.apache.org/jira/browse/SQOOP-600
>
>
> Diffs
> -----
>
>   src/java/org/apache/sqoop/mapreduce/HBaseImportJob.java 6a784d2
>
> Diff: https://reviews.apache.org/r/7014/diff/
>
>
> Testing
> -------
>
> I've done only manual testing on HBase 0.94. Automatic tests are disabled in different profiles other than hadoopversion={20,100} and I'll work on getting them working later.
>
>
> Thanks,
>
> Jarek Cecho
>
>

> On Sept. 12, 2012, 8:45 p.m., Abhijeet Gaikwad wrote:
> > ant test (100): Success. I think the Hbase tests are disabled for hadoopversion 100 as well. Let me know.
> > ant test (20): Hbase tests timed out for me(TestHbaseImport and TestHbaseQueryImport). These don't seem to be a regression as they are failing on my m/c for current trunk as well. If they work for you, I have a ship it.
> >
> >

Thank you sir for your time. I've attached newest version of the patch to the JIRA (small change that Cheolsoo has suggested), but I'm having difficulties to upload it to the review board. Please feel free to commit it.
- Jarek
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/7014/#review11429
-----------------------------------------------------------
On Sept. 11, 2012, 11:03 a.m., Jarek Cecho wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7014/
> -----------------------------------------------------------
>
> (Updated Sept. 11, 2012, 11:03 a.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> Problem is that we're creating Table descriptor locally, which is not importing remote table families automatically. I've enhanced the code to retrieve remote table descriptor prior checking if the table family is present.
>
>
> This addresses bug SQOOP-600.
>     https://issues.apache.org/jira/browse/SQOOP-600
>
>
> Diffs
> -----
>
>   src/java/org/apache/sqoop/mapreduce/HBaseImportJob.java 6a784d2
>
> Diff: https://reviews.apache.org/r/7014/diff/
>
>
> Testing
> -------
>
> I've done only manual testing on HBase 0.94. Automatic tests are disabled in different profiles other than hadoopversion={20,100} and I'll work on getting them working later.
>
>
> Thanks,
>
> Jarek Cecho
>
>

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

Ship it!
- Abhijeet Gaikwad
On Sept. 11, 2012, 11:03 a.m., Jarek Cecho wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/7014/
> -----------------------------------------------------------
>
> (Updated Sept. 11, 2012, 11:03 a.m.)
>
>
> Review request for Sqoop.
>
>
> Description
> -------
>
> Problem is that we're creating Table descriptor locally, which is not importing remote table families automatically. I've enhanced the code to retrieve remote table descriptor prior checking if the table family is present.
>
>
> This addresses bug SQOOP-600.
>     https://issues.apache.org/jira/browse/SQOOP-600
>
>
> Diffs
> -----
>
>   src/java/org/apache/sqoop/mapreduce/HBaseImportJob.java 6a784d2
>
> Diff: https://reviews.apache.org/r/7014/diff/
>
>
> Testing
> -------
>
> I've done only manual testing on HBase 0.94. Automatic tests are disabled in different profiles other than hadoopversion={20,100} and I'll work on getting them working later.
>
>
> Thanks,
>
> Jarek Cecho
>
>