You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hbase.apache.org by Jean-Daniel Cryans <jd...@apache.org> on 2011/09/26 18:55:14 UTC

Re: Better Javadoc HTableDescriptor

Be my guest!

J-D

On Sat, Sep 24, 2011 at 9:16 AM, Akash <th...@gmail.com> wrote:
> Hi,
>     I was just lookin at the HTableDescriptor and realized it has very minimal javadoc. Considering this is an admin API I feel there should be better javadoc. If there's no Jira already open I would go ahead and file it. Thanks
>
> Sent from my iPhone

Re: Better Javadoc HTableDescriptor

Posted by Akash Ashok <th...@gmail.com>.
I totally agree that its not at all accepted if it causes compilation
failures on upgrades. But the very name of the function is very misleading
considering the fact that there is a .META. region and its very hard to
distinguish between  isMetaRegion() and isMetaTable(). We could definitely
add the javadoc which anyways has to be done. But I feel we have to also
deprecate and add a new method which would aptly be named according to what
it does. This way it doesn't cause compilation failures and also ensures
that over time users of Hbase will eventually start realizing that its
deprecated and start changing it to use the new method.

We could remove the method when the community deems fit to do so.

Cheers,
Akash A

On Sat, Oct 1, 2011 at 10:32 PM, Doug Meil <do...@explorysmedical.com>wrote:

>
> Be very careful about code-changes that causes compilation failures upon
> upgrade.  Once in a while that may happen on a major upgrade, but it had
> better be for a very good, explainable, reason that the community is
> behind.
>
>
> A fourth option is to add Javadoc to the explaining what this method does
> and not add any new methods.
>
>
> On 10/1/11 8:02 AM, "Akash Ashok" <th...@gmail.com> wrote:
>
> >While I was doing this I found out that there is a method in
> >HTableDescriptor called isMetaTable() which basically checks if the table
> >is
> >a catalog table and the name kind of misleading. I am planning to rename
> >this method to isCatalogTable().
> >
> >1. Is it ok to do this renaming considering that this is a client side
> >API/
> >Admin API might lead to a lot of changes on client side for people already
> >using this method?
> >
> >2. If yes, is it ok to do it under the same JIRA HBase-4486 or should
> >another another JIRA be file for this ?
> >
> >3. If no, could this method be deprecated and another new method
> >isCatalogTable() be added ?
> >
> >Cheers,
> >Akash A
> >
> >On Mon, Sep 26, 2011 at 10:25 PM, Jean-Daniel Cryans
> ><jd...@apache.org>wrote:
> >
> >> Be my guest!
> >>
> >> J-D
> >>
> >> On Sat, Sep 24, 2011 at 9:16 AM, Akash <th...@gmail.com> wrote:
> >> > Hi,
> >> >     I was just lookin at the HTableDescriptor and realized it has very
> >> minimal javadoc. Considering this is an admin API I feel there should be
> >> better javadoc. If there's no Jira already open I would go ahead and
> >>file
> >> it. Thanks
> >> >
> >> > Sent from my iPhone
> >>
>
>

Re: Better Javadoc HTableDescriptor

Posted by Akash Ashok <th...@gmail.com>.
Hi,
Looking the the two methods
1. isMetaTable() - returns if table is meta and not root, i.e returns if the
table represents .META.

    public boolean isMetaTable() {
        return isMetaRegion() && !isRootRegion();
    }

2. isMetaTable(String tableName) returns if a table is either .META. or
-ROOT-

    public static boolean isMetaTable(final byte [] tableName) {
        return Bytes.equals(tableName, HConstants.ROOT_TABLE_NAME) ||
          Bytes.equals(tableName, HConstants.META_TABLE_NAME);
    }

This overload of the function is not at all ok. Same method behaving very
differently in two situations is very confusing even with the detailed
Javadoc in place.

I'd suggest changing the implementation of 2 to return only if the table is
.META.

Like to hear your thoughts

Cheers,
Akash A

On Sun, Oct 2, 2011 at 2:07 AM, Shumin Wu <sh...@gmail.com> wrote:

> Hello All,
>
> I am new here. So very nice to meet you guys.
>
> Here are my thoughts on this issue. I agree with Akash that the name of
> this
> method would confuse the user as there is a .META. notion that means
> something else. So why don't we create a function alias for isMetaTable()
> as
> a temporary solution? When users gradually migrate to the new API
> (isCatalogTable), we can then deprecate the isMetaTable function. Of course
> the javadoc will need to well explain it.
>
>
> Code Snippet:
>
>  public boolean isMetaTable() {
>    return isCatalogTable();
>  }
>
>  public boolean isCatalogTable() {
>    return isMetaRegion() && !isRootRegion();
>  }
>
> Thanks,
> Shumin
>
> On Sat, Oct 1, 2011 at 1:16 PM, Akash Ashok <th...@gmail.com>
> wrote:
>
> > Ok Thanks. I shall just add the javadoc. I've already opened a JIRA for
> > improving the javadoc
> >
> > https://issues.apache.org/jira/browse/HBASE-4486
> >
> > Cheers,
> > Akash A
> >
> > On Sun, Oct 2, 2011 at 12:21 AM, Dhruba Borthakur <dh...@gmail.com>
> > wrote:
> >
> > > +1 for Option 4.
> > >
> > > -dhruba
> > >
> > > On Sat, Oct 1, 2011 at 11:26 AM, Ted Yu <yu...@gmail.com> wrote:
> > >
> > > > I prefer the fourth option.
> > > > Only truly broken APIs should be removed.
> > > >
> > > > Cheers
> > > >
> > > > On Oct 1, 2011, at 10:02 AM, Doug Meil <
> doug.meil@explorysmedical.com>
> > > > wrote:
> > > >
> > > > >
> > > > > Be very careful about code-changes that causes compilation failures
> > > upon
> > > > > upgrade.  Once in a while that may happen on a major upgrade, but
> it
> > > had
> > > > > better be for a very good, explainable, reason that the community
> is
> > > > > behind.
> > > > >
> > > > >
> > > > > A fourth option is to add Javadoc to the explaining what this
> method
> > > does
> > > > > and not add any new methods.
> > > > >
> > > > >
> > > > > On 10/1/11 8:02 AM, "Akash Ashok" <th...@gmail.com> wrote:
> > > > >
> > > > >> While I was doing this I found out that there is a method in
> > > > >> HTableDescriptor called isMetaTable() which basically checks if
> the
> > > > table
> > > > >> is
> > > > >> a catalog table and the name kind of misleading. I am planning to
> > > rename
> > > > >> this method to isCatalogTable().
> > > > >>
> > > > >> 1. Is it ok to do this renaming considering that this is a client
> > side
> > > > >> API/
> > > > >> Admin API might lead to a lot of changes on client side for people
> > > > already
> > > > >> using this method?
> > > > >>
> > > > >> 2. If yes, is it ok to do it under the same JIRA HBase-4486 or
> > should
> > > > >> another another JIRA be file for this ?
> > > > >>
> > > > >> 3. If no, could this method be deprecated and another new method
> > > > >> isCatalogTable() be added ?
> > > > >>
> > > > >> Cheers,
> > > > >> Akash A
> > > > >>
> > > > >> On Mon, Sep 26, 2011 at 10:25 PM, Jean-Daniel Cryans
> > > > >> <jd...@apache.org>wrote:
> > > > >>
> > > > >>> Be my guest!
> > > > >>>
> > > > >>> J-D
> > > > >>>
> > > > >>> On Sat, Sep 24, 2011 at 9:16 AM, Akash <th...@gmail.com>
> > > wrote:
> > > > >>>> Hi,
> > > > >>>>    I was just lookin at the HTableDescriptor and realized it has
> > > very
> > > > >>> minimal javadoc. Considering this is an admin API I feel there
> > should
> > > > be
> > > > >>> better javadoc. If there's no Jira already open I would go ahead
> > and
> > > > >>> file
> > > > >>> it. Thanks
> > > > >>>>
> > > > >>>> Sent from my iPhone
> > > > >>>
> > > > >
> > > >
> > >
> > >
> > >
> > > --
> > > Connect to me at http://www.facebook.com/dhruba
> > >
> >
>

Re: Better Javadoc HTableDescriptor

Posted by Shumin Wu <sh...@gmail.com>.
Hello All,

I am new here. So very nice to meet you guys.

Here are my thoughts on this issue. I agree with Akash that the name of this
method would confuse the user as there is a .META. notion that means
something else. So why don't we create a function alias for isMetaTable() as
a temporary solution? When users gradually migrate to the new API
(isCatalogTable), we can then deprecate the isMetaTable function. Of course
the javadoc will need to well explain it.


Code Snippet:

  public boolean isMetaTable() {
    return isCatalogTable();
  }

  public boolean isCatalogTable() {
    return isMetaRegion() && !isRootRegion();
 }

Thanks,
Shumin

On Sat, Oct 1, 2011 at 1:16 PM, Akash Ashok <th...@gmail.com> wrote:

> Ok Thanks. I shall just add the javadoc. I've already opened a JIRA for
> improving the javadoc
>
> https://issues.apache.org/jira/browse/HBASE-4486
>
> Cheers,
> Akash A
>
> On Sun, Oct 2, 2011 at 12:21 AM, Dhruba Borthakur <dh...@gmail.com>
> wrote:
>
> > +1 for Option 4.
> >
> > -dhruba
> >
> > On Sat, Oct 1, 2011 at 11:26 AM, Ted Yu <yu...@gmail.com> wrote:
> >
> > > I prefer the fourth option.
> > > Only truly broken APIs should be removed.
> > >
> > > Cheers
> > >
> > > On Oct 1, 2011, at 10:02 AM, Doug Meil <do...@explorysmedical.com>
> > > wrote:
> > >
> > > >
> > > > Be very careful about code-changes that causes compilation failures
> > upon
> > > > upgrade.  Once in a while that may happen on a major upgrade, but it
> > had
> > > > better be for a very good, explainable, reason that the community is
> > > > behind.
> > > >
> > > >
> > > > A fourth option is to add Javadoc to the explaining what this method
> > does
> > > > and not add any new methods.
> > > >
> > > >
> > > > On 10/1/11 8:02 AM, "Akash Ashok" <th...@gmail.com> wrote:
> > > >
> > > >> While I was doing this I found out that there is a method in
> > > >> HTableDescriptor called isMetaTable() which basically checks if the
> > > table
> > > >> is
> > > >> a catalog table and the name kind of misleading. I am planning to
> > rename
> > > >> this method to isCatalogTable().
> > > >>
> > > >> 1. Is it ok to do this renaming considering that this is a client
> side
> > > >> API/
> > > >> Admin API might lead to a lot of changes on client side for people
> > > already
> > > >> using this method?
> > > >>
> > > >> 2. If yes, is it ok to do it under the same JIRA HBase-4486 or
> should
> > > >> another another JIRA be file for this ?
> > > >>
> > > >> 3. If no, could this method be deprecated and another new method
> > > >> isCatalogTable() be added ?
> > > >>
> > > >> Cheers,
> > > >> Akash A
> > > >>
> > > >> On Mon, Sep 26, 2011 at 10:25 PM, Jean-Daniel Cryans
> > > >> <jd...@apache.org>wrote:
> > > >>
> > > >>> Be my guest!
> > > >>>
> > > >>> J-D
> > > >>>
> > > >>> On Sat, Sep 24, 2011 at 9:16 AM, Akash <th...@gmail.com>
> > wrote:
> > > >>>> Hi,
> > > >>>>    I was just lookin at the HTableDescriptor and realized it has
> > very
> > > >>> minimal javadoc. Considering this is an admin API I feel there
> should
> > > be
> > > >>> better javadoc. If there's no Jira already open I would go ahead
> and
> > > >>> file
> > > >>> it. Thanks
> > > >>>>
> > > >>>> Sent from my iPhone
> > > >>>
> > > >
> > >
> >
> >
> >
> > --
> > Connect to me at http://www.facebook.com/dhruba
> >
>

Re: Better Javadoc HTableDescriptor

Posted by Akash Ashok <th...@gmail.com>.
Ok Thanks. I shall just add the javadoc. I've already opened a JIRA for
improving the javadoc

https://issues.apache.org/jira/browse/HBASE-4486

Cheers,
Akash A

On Sun, Oct 2, 2011 at 12:21 AM, Dhruba Borthakur <dh...@gmail.com> wrote:

> +1 for Option 4.
>
> -dhruba
>
> On Sat, Oct 1, 2011 at 11:26 AM, Ted Yu <yu...@gmail.com> wrote:
>
> > I prefer the fourth option.
> > Only truly broken APIs should be removed.
> >
> > Cheers
> >
> > On Oct 1, 2011, at 10:02 AM, Doug Meil <do...@explorysmedical.com>
> > wrote:
> >
> > >
> > > Be very careful about code-changes that causes compilation failures
> upon
> > > upgrade.  Once in a while that may happen on a major upgrade, but it
> had
> > > better be for a very good, explainable, reason that the community is
> > > behind.
> > >
> > >
> > > A fourth option is to add Javadoc to the explaining what this method
> does
> > > and not add any new methods.
> > >
> > >
> > > On 10/1/11 8:02 AM, "Akash Ashok" <th...@gmail.com> wrote:
> > >
> > >> While I was doing this I found out that there is a method in
> > >> HTableDescriptor called isMetaTable() which basically checks if the
> > table
> > >> is
> > >> a catalog table and the name kind of misleading. I am planning to
> rename
> > >> this method to isCatalogTable().
> > >>
> > >> 1. Is it ok to do this renaming considering that this is a client side
> > >> API/
> > >> Admin API might lead to a lot of changes on client side for people
> > already
> > >> using this method?
> > >>
> > >> 2. If yes, is it ok to do it under the same JIRA HBase-4486 or should
> > >> another another JIRA be file for this ?
> > >>
> > >> 3. If no, could this method be deprecated and another new method
> > >> isCatalogTable() be added ?
> > >>
> > >> Cheers,
> > >> Akash A
> > >>
> > >> On Mon, Sep 26, 2011 at 10:25 PM, Jean-Daniel Cryans
> > >> <jd...@apache.org>wrote:
> > >>
> > >>> Be my guest!
> > >>>
> > >>> J-D
> > >>>
> > >>> On Sat, Sep 24, 2011 at 9:16 AM, Akash <th...@gmail.com>
> wrote:
> > >>>> Hi,
> > >>>>    I was just lookin at the HTableDescriptor and realized it has
> very
> > >>> minimal javadoc. Considering this is an admin API I feel there should
> > be
> > >>> better javadoc. If there's no Jira already open I would go ahead and
> > >>> file
> > >>> it. Thanks
> > >>>>
> > >>>> Sent from my iPhone
> > >>>
> > >
> >
>
>
>
> --
> Connect to me at http://www.facebook.com/dhruba
>

Re: Better Javadoc HTableDescriptor

Posted by Dhruba Borthakur <dh...@gmail.com>.
+1 for Option 4.

-dhruba

On Sat, Oct 1, 2011 at 11:26 AM, Ted Yu <yu...@gmail.com> wrote:

> I prefer the fourth option.
> Only truly broken APIs should be removed.
>
> Cheers
>
> On Oct 1, 2011, at 10:02 AM, Doug Meil <do...@explorysmedical.com>
> wrote:
>
> >
> > Be very careful about code-changes that causes compilation failures upon
> > upgrade.  Once in a while that may happen on a major upgrade, but it had
> > better be for a very good, explainable, reason that the community is
> > behind.
> >
> >
> > A fourth option is to add Javadoc to the explaining what this method does
> > and not add any new methods.
> >
> >
> > On 10/1/11 8:02 AM, "Akash Ashok" <th...@gmail.com> wrote:
> >
> >> While I was doing this I found out that there is a method in
> >> HTableDescriptor called isMetaTable() which basically checks if the
> table
> >> is
> >> a catalog table and the name kind of misleading. I am planning to rename
> >> this method to isCatalogTable().
> >>
> >> 1. Is it ok to do this renaming considering that this is a client side
> >> API/
> >> Admin API might lead to a lot of changes on client side for people
> already
> >> using this method?
> >>
> >> 2. If yes, is it ok to do it under the same JIRA HBase-4486 or should
> >> another another JIRA be file for this ?
> >>
> >> 3. If no, could this method be deprecated and another new method
> >> isCatalogTable() be added ?
> >>
> >> Cheers,
> >> Akash A
> >>
> >> On Mon, Sep 26, 2011 at 10:25 PM, Jean-Daniel Cryans
> >> <jd...@apache.org>wrote:
> >>
> >>> Be my guest!
> >>>
> >>> J-D
> >>>
> >>> On Sat, Sep 24, 2011 at 9:16 AM, Akash <th...@gmail.com> wrote:
> >>>> Hi,
> >>>>    I was just lookin at the HTableDescriptor and realized it has very
> >>> minimal javadoc. Considering this is an admin API I feel there should
> be
> >>> better javadoc. If there's no Jira already open I would go ahead and
> >>> file
> >>> it. Thanks
> >>>>
> >>>> Sent from my iPhone
> >>>
> >
>



-- 
Connect to me at http://www.facebook.com/dhruba

Re: Better Javadoc HTableDescriptor

Posted by Ted Yu <yu...@gmail.com>.
I prefer the fourth option. 
Only truly broken APIs should be removed. 

Cheers

On Oct 1, 2011, at 10:02 AM, Doug Meil <do...@explorysmedical.com> wrote:

> 
> Be very careful about code-changes that causes compilation failures upon
> upgrade.  Once in a while that may happen on a major upgrade, but it had
> better be for a very good, explainable, reason that the community is
> behind.
> 
> 
> A fourth option is to add Javadoc to the explaining what this method does
> and not add any new methods.
> 
> 
> On 10/1/11 8:02 AM, "Akash Ashok" <th...@gmail.com> wrote:
> 
>> While I was doing this I found out that there is a method in
>> HTableDescriptor called isMetaTable() which basically checks if the table
>> is
>> a catalog table and the name kind of misleading. I am planning to rename
>> this method to isCatalogTable().
>> 
>> 1. Is it ok to do this renaming considering that this is a client side
>> API/
>> Admin API might lead to a lot of changes on client side for people already
>> using this method?
>> 
>> 2. If yes, is it ok to do it under the same JIRA HBase-4486 or should
>> another another JIRA be file for this ?
>> 
>> 3. If no, could this method be deprecated and another new method
>> isCatalogTable() be added ?
>> 
>> Cheers,
>> Akash A
>> 
>> On Mon, Sep 26, 2011 at 10:25 PM, Jean-Daniel Cryans
>> <jd...@apache.org>wrote:
>> 
>>> Be my guest!
>>> 
>>> J-D
>>> 
>>> On Sat, Sep 24, 2011 at 9:16 AM, Akash <th...@gmail.com> wrote:
>>>> Hi,
>>>>    I was just lookin at the HTableDescriptor and realized it has very
>>> minimal javadoc. Considering this is an admin API I feel there should be
>>> better javadoc. If there's no Jira already open I would go ahead and
>>> file
>>> it. Thanks
>>>> 
>>>> Sent from my iPhone
>>> 
> 

Re: Better Javadoc HTableDescriptor

Posted by Doug Meil <do...@explorysmedical.com>.
Be very careful about code-changes that causes compilation failures upon
upgrade.  Once in a while that may happen on a major upgrade, but it had
better be for a very good, explainable, reason that the community is
behind.


A fourth option is to add Javadoc to the explaining what this method does
and not add any new methods.


On 10/1/11 8:02 AM, "Akash Ashok" <th...@gmail.com> wrote:

>While I was doing this I found out that there is a method in
>HTableDescriptor called isMetaTable() which basically checks if the table
>is
>a catalog table and the name kind of misleading. I am planning to rename
>this method to isCatalogTable().
>
>1. Is it ok to do this renaming considering that this is a client side
>API/
>Admin API might lead to a lot of changes on client side for people already
>using this method?
>
>2. If yes, is it ok to do it under the same JIRA HBase-4486 or should
>another another JIRA be file for this ?
>
>3. If no, could this method be deprecated and another new method
>isCatalogTable() be added ?
>
>Cheers,
>Akash A
>
>On Mon, Sep 26, 2011 at 10:25 PM, Jean-Daniel Cryans
><jd...@apache.org>wrote:
>
>> Be my guest!
>>
>> J-D
>>
>> On Sat, Sep 24, 2011 at 9:16 AM, Akash <th...@gmail.com> wrote:
>> > Hi,
>> >     I was just lookin at the HTableDescriptor and realized it has very
>> minimal javadoc. Considering this is an admin API I feel there should be
>> better javadoc. If there's no Jira already open I would go ahead and
>>file
>> it. Thanks
>> >
>> > Sent from my iPhone
>>


Re: Better Javadoc HTableDescriptor

Posted by Akash Ashok <th...@gmail.com>.
While I was doing this I found out that there is a method in
HTableDescriptor called isMetaTable() which basically checks if the table is
a catalog table and the name kind of misleading. I am planning to rename
this method to isCatalogTable().

1. Is it ok to do this renaming considering that this is a client side API/
Admin API might lead to a lot of changes on client side for people already
using this method?

2. If yes, is it ok to do it under the same JIRA HBase-4486 or should
another another JIRA be file for this ?

3. If no, could this method be deprecated and another new method
isCatalogTable() be added ?

Cheers,
Akash A

On Mon, Sep 26, 2011 at 10:25 PM, Jean-Daniel Cryans <jd...@apache.org>wrote:

> Be my guest!
>
> J-D
>
> On Sat, Sep 24, 2011 at 9:16 AM, Akash <th...@gmail.com> wrote:
> > Hi,
> >     I was just lookin at the HTableDescriptor and realized it has very
> minimal javadoc. Considering this is an admin API I feel there should be
> better javadoc. If there's no Jira already open I would go ahead and file
> it. Thanks
> >
> > Sent from my iPhone
>