[ 
https://issues.apache.org/jira/browse/HBASE-11879?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14140069#comment-14140069
 ] 

stack commented on HBASE-11879:
-------------------------------

Took a quick look. I think it looking good.  Looks like old MR jobs will 
continue to work.

nits: Fix spelling in + *          
ConnectionFactory.createConnectin(HBaseConfiguration.create(job));
nits: When you @Deprecated a method, you can add a @deprecated to javadoc too 
and qualify it with what you would have the user call instead.  You actually 
have javadoc already doing '* Use setTable'.  Instead making '* @deprecated Use 
setTable instead'.  Minor.

Is this right (in mapred package -- seems right in mapreduce package)?

-  public void setHTable(Table htable) {
-    Configuration conf = htable.getConfiguration();
+  public void setHTable(Table table) {

Should there be a deprecation of setHTable going on here?

What was wrong w/ your 2a suggestion?  That looked nice.  Let this class 
internally go get regionlocators?

Otherwise patch is looking good.

bq. Does it make sense to migrate getRegionLocations() and its dependencies to 
use the new Table/RegionLocator/Connection/Admin interfaces?

That would make sense.  In another JIRA?

Good stuff.


> Change TableInputFormatBase to take interface arguments
> -------------------------------------------------------
>
>                 Key: HBASE-11879
>                 URL: https://issues.apache.org/jira/browse/HBASE-11879
>             Project: HBase
>          Issue Type: Improvement
>            Reporter: Carter
>            Assignee: Solomon Duskis
>             Fix For: 0.99.1
>
>         Attachments: HBASE_11879.patch
>
>
> As part of the ongoing interface abstraction work, I'm now investigating 
> {{TableInputFormatBase}}, which has two methods that break encapsulation:
> {code}
> protected HTable getHTable();
> protected void setHTable(HTable table);
> {code}
> While these are protected methods, the base @InterfaceAudience.Public is 
> abstract, meaning that it supports extension by user code.
> I propose deprecating these two methods and replacing them with these four, 
> once the Table interface is merged:
> {code}
> protected Table getTable();
> protected void setTable(Table table);
> protected RegionLocator getRegionLocator();
> protected void setRegionLocator(RegionLocator regionLocator);
> {code}
> Since users will frequently call {{setTable}} and {{setRegionLocator}} 
> together, it probably also makes sense to add the following convenience 
> method:
> {code}
> protected void setTableAndRegionLocator(Table table, RegionLocator 
> regionLocator);
> {code}



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to