You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@ignite.apache.org by Stanislav Lukyanov <st...@gmail.com> on 2021/10/06 09:46:24 UTC

API Proposal: Declare IgniteClient::close that throws no exceptions (IGNITE-15688)

Hi Igniters,

I found the following usability issue with java thin client API.

Whenever you do `try (IgniteClient client = Ignition.startClient(cfg))`, you're forced to declare `catch (Exception e)`.

This is because IgniteClient interface currently doesn't override close() from AutoClosable. Because of that, it inherits `close() throws Exception`.
 
In fact, this shouldn't be required. `TcpIgniteClient implements IgniteClient` currently throws Exception but it doesn't need to - its code doesn't throw any checked exceptions.
 
Proposal:
	• Add `@Overrides public void close()` with no `throws` to IgniteClient.
	• Remove `throws Exception` from `TcpIgniteClient::close`
Note: this change is fully backwards-compatible. It is legal to narrow the scope of `throws` in a new version of a method.

I plan to do this in https://issues.apache.org/jira/browse/IGNITE-15688.

Any objections?

Thanks,
Stan

Re: API Proposal: Declare IgniteClient::close that throws no exceptions (IGNITE-15688)

Posted by Stanislav Lukyanov <st...@gmail.com>.

Patch, PR and visa are in the ticket.

Igor, please review and merge when you're ready.

Thanks,
Stan

> On 6 Oct 2021, at 12:54, Igor Sapego <is...@apache.org> wrote:
> 
> Sounds good, no objections from my side.
> 
> Best Regards,
> Igor
> 
> 
> On Wed, Oct 6, 2021 at 11:46 AM Stanislav Lukyanov <st...@gmail.com>
> wrote:
> 
>> Hi Igniters,
>> 
>> I found the following usability issue with java thin client API.
>> 
>> Whenever you do `try (IgniteClient client = Ignition.startClient(cfg))`,
>> you're forced to declare `catch (Exception e)`.
>> 
>> This is because IgniteClient interface currently doesn't override close()
>> from AutoClosable. Because of that, it inherits `close() throws Exception`.
>> 
>> In fact, this shouldn't be required. `TcpIgniteClient implements
>> IgniteClient` currently throws Exception but it doesn't need to - its code
>> doesn't throw any checked exceptions.
>> 
>> Proposal:
>>        • Add `@Overrides public void close()` with no `throws` to
>> IgniteClient.
>>        • Remove `throws Exception` from `TcpIgniteClient::close`
>> Note: this change is fully backwards-compatible. It is legal to narrow the
>> scope of `throws` in a new version of a method.
>> 
>> I plan to do this in https://issues.apache.org/jira/browse/IGNITE-15688.
>> 
>> Any objections?
>> 
>> Thanks,
>> Stan

Re: API Proposal: Declare IgniteClient::close that throws no exceptions (IGNITE-15688)

Posted by Igor Sapego <is...@apache.org>.

Sounds good, no objections from my side.

Best Regards,
Igor


On Wed, Oct 6, 2021 at 11:46 AM Stanislav Lukyanov <st...@gmail.com>
wrote:

> Hi Igniters,
>
> I found the following usability issue with java thin client API.
>
> Whenever you do `try (IgniteClient client = Ignition.startClient(cfg))`,
> you're forced to declare `catch (Exception e)`.
>
> This is because IgniteClient interface currently doesn't override close()
> from AutoClosable. Because of that, it inherits `close() throws Exception`.
>
> In fact, this shouldn't be required. `TcpIgniteClient implements
> IgniteClient` currently throws Exception but it doesn't need to - its code
> doesn't throw any checked exceptions.
>
> Proposal:
>         • Add `@Overrides public void close()` with no `throws` to
> IgniteClient.
>         • Remove `throws Exception` from `TcpIgniteClient::close`
> Note: this change is fully backwards-compatible. It is legal to narrow the
> scope of `throws` in a new version of a method.
>
> I plan to do this in https://issues.apache.org/jira/browse/IGNITE-15688.
>
> Any objections?
>
> Thanks,
> Stan