You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@hbase.apache.org by "张铎 (Duo Zhang)" <pa...@gmail.com> on 2020/12/01 14:44:36 UTC

Re: [DISCUSS] The breaking changes in HBASE-24966

If no other objections tomorrow, will follow what Sean proposed above, to
include the patch in 3.0.0 and mark the issue as incompatible change.

Thanks.

张铎(Duo Zhang) <pa...@gmail.com> 于2020年11月30日周一 上午11:04写道:

> I think it could introduce compilation error when removing the throws part
> of a method signature. As if there is no exception thrown but you have a
> 'try...catch' then there will be a compilation error...
>
> It requires a code change sometimes but anyway, you just need to remove
> the 'try...catch', no other big impacts. So for me I would also like to
> mark it as incomplete and change it directly in 3.0.0.
>
> Thanks.
>
> Sean Busbey <bu...@apache.org> 于2020年11月30日周一 上午2:51写道:
>
>> I think we should change what they throw directly and label it
>> incompatible. I think this is in line with our previous expectation
>> setting
>> about how we'll handle mistakes in the API.
>>
>> That change would be source incompatible but would still be binary
>> compatible.
>>
>> I think we should do it in a major release. esp since there's not a way in
>> Java to say "this deprecation is just about the thrown exceptions" and it
>> will be awkward to write code that is source compatible with the existing
>> api and with the exception removed.
>>
>> On Sat, Nov 28, 2020, 22:07 张铎(Duo Zhang) <pa...@gmail.com> wrote:
>>
>> > In HBASE-24966, we found that in AsyncTableRegionLocator, we
>> accidentally
>> > declared 3 methods
>> >
>> > getStartKeys
>> > getEndKeys
>> > getStartEndKeys
>> >
>> > to throw IOException directly.
>> >
>> > This should be a copy paste mistake, as typically, for a method which
>> > returns CompletableFuture, the exception should be returned through the
>> > CompletableFuture, and this is exactly the behavior of these methods.
>> >
>> > So the actual problem is only that we have a wrong method signature. but
>> > since this interface is IA.Public, and it has already been included in
>> > several releases, according to our compatibility rule, we can not just
>> > remove the throws part from the method. Instead, we need to deprecate
>> them
>> > and create new methods. But there will be another problem that we want
>> to
>> > align the method names between the sync and async client, so if we
>> change
>> > the names of the methods, we'd better also change the name of methods
>> for
>> > sync client, which will make our users do more unnecessary work.
>> >
>> > So here I want to discuss that, since we all know that, this is a
>> mistake,
>> > and the methods will never throw IOException directly, is it OK for us
>> to
>> > just remove the throws part and tell users directly that this is a
>> mistake,
>> > and release it in the next minor release or a major release as an
>> > incompatible change?
>> >
>> > Thanks.
>> >
>>
>

Re: [DISCUSS] The breaking changes in HBASE-24966

Posted by Nick Dimiduk <nd...@apache.org>.
On Tue, Dec 1, 2020 at 6:45 AM 张铎(Duo Zhang) <pa...@gmail.com> wrote:
>
> If no other objections tomorrow, will follow what Sean proposed above, to
> include the patch in 3.0.0 and mark the issue as incompatible change.

Belated +1.

> 张铎(Duo Zhang) <pa...@gmail.com> 于2020年11月30日周一 上午11:04写道:
>
> > I think it could introduce compilation error when removing the throws part
> > of a method signature. As if there is no exception thrown but you have a
> > 'try...catch' then there will be a compilation error...
> >
> > It requires a code change sometimes but anyway, you just need to remove
> > the 'try...catch', no other big impacts. So for me I would also like to
> > mark it as incomplete and change it directly in 3.0.0.
> >
> > Thanks.
> >
> > Sean Busbey <bu...@apache.org> 于2020年11月30日周一 上午2:51写道:
> >
> >> I think we should change what they throw directly and label it
> >> incompatible. I think this is in line with our previous expectation
> >> setting
> >> about how we'll handle mistakes in the API.
> >>
> >> That change would be source incompatible but would still be binary
> >> compatible.
> >>
> >> I think we should do it in a major release. esp since there's not a way in
> >> Java to say "this deprecation is just about the thrown exceptions" and it
> >> will be awkward to write code that is source compatible with the existing
> >> api and with the exception removed.
> >>
> >> On Sat, Nov 28, 2020, 22:07 张铎(Duo Zhang) <pa...@gmail.com> wrote:
> >>
> >> > In HBASE-24966, we found that in AsyncTableRegionLocator, we
> >> accidentally
> >> > declared 3 methods
> >> >
> >> > getStartKeys
> >> > getEndKeys
> >> > getStartEndKeys
> >> >
> >> > to throw IOException directly.
> >> >
> >> > This should be a copy paste mistake, as typically, for a method which
> >> > returns CompletableFuture, the exception should be returned through the
> >> > CompletableFuture, and this is exactly the behavior of these methods.
> >> >
> >> > So the actual problem is only that we have a wrong method signature. but
> >> > since this interface is IA.Public, and it has already been included in
> >> > several releases, according to our compatibility rule, we can not just
> >> > remove the throws part from the method. Instead, we need to deprecate
> >> them
> >> > and create new methods. But there will be another problem that we want
> >> to
> >> > align the method names between the sync and async client, so if we
> >> change
> >> > the names of the methods, we'd better also change the name of methods
> >> for
> >> > sync client, which will make our users do more unnecessary work.
> >> >
> >> > So here I want to discuss that, since we all know that, this is a
> >> mistake,
> >> > and the methods will never throw IOException directly, is it OK for us
> >> to
> >> > just remove the throws part and tell users directly that this is a
> >> mistake,
> >> > and release it in the next minor release or a major release as an
> >> > incompatible change?
> >> >
> >> > Thanks.
> >> >
> >>
> >

Re: [DISCUSS] The breaking changes in HBASE-24966

Posted by Nick Dimiduk <nd...@apache.org>.
On Tue, Dec 1, 2020 at 6:45 AM 张铎(Duo Zhang) <pa...@gmail.com> wrote:
>
> If no other objections tomorrow, will follow what Sean proposed above, to
> include the patch in 3.0.0 and mark the issue as incompatible change.

Belated +1.

> 张铎(Duo Zhang) <pa...@gmail.com> 于2020年11月30日周一 上午11:04写道:
>
> > I think it could introduce compilation error when removing the throws part
> > of a method signature. As if there is no exception thrown but you have a
> > 'try...catch' then there will be a compilation error...
> >
> > It requires a code change sometimes but anyway, you just need to remove
> > the 'try...catch', no other big impacts. So for me I would also like to
> > mark it as incomplete and change it directly in 3.0.0.
> >
> > Thanks.
> >
> > Sean Busbey <bu...@apache.org> 于2020年11月30日周一 上午2:51写道:
> >
> >> I think we should change what they throw directly and label it
> >> incompatible. I think this is in line with our previous expectation
> >> setting
> >> about how we'll handle mistakes in the API.
> >>
> >> That change would be source incompatible but would still be binary
> >> compatible.
> >>
> >> I think we should do it in a major release. esp since there's not a way in
> >> Java to say "this deprecation is just about the thrown exceptions" and it
> >> will be awkward to write code that is source compatible with the existing
> >> api and with the exception removed.
> >>
> >> On Sat, Nov 28, 2020, 22:07 张铎(Duo Zhang) <pa...@gmail.com> wrote:
> >>
> >> > In HBASE-24966, we found that in AsyncTableRegionLocator, we
> >> accidentally
> >> > declared 3 methods
> >> >
> >> > getStartKeys
> >> > getEndKeys
> >> > getStartEndKeys
> >> >
> >> > to throw IOException directly.
> >> >
> >> > This should be a copy paste mistake, as typically, for a method which
> >> > returns CompletableFuture, the exception should be returned through the
> >> > CompletableFuture, and this is exactly the behavior of these methods.
> >> >
> >> > So the actual problem is only that we have a wrong method signature. but
> >> > since this interface is IA.Public, and it has already been included in
> >> > several releases, according to our compatibility rule, we can not just
> >> > remove the throws part from the method. Instead, we need to deprecate
> >> them
> >> > and create new methods. But there will be another problem that we want
> >> to
> >> > align the method names between the sync and async client, so if we
> >> change
> >> > the names of the methods, we'd better also change the name of methods
> >> for
> >> > sync client, which will make our users do more unnecessary work.
> >> >
> >> > So here I want to discuss that, since we all know that, this is a
> >> mistake,
> >> > and the methods will never throw IOException directly, is it OK for us
> >> to
> >> > just remove the throws part and tell users directly that this is a
> >> mistake,
> >> > and release it in the next minor release or a major release as an
> >> > incompatible change?
> >> >
> >> > Thanks.
> >> >
> >>
> >