You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@thrift.apache.org by John Sirois <js...@apache.org> on 2016/02/08 20:40:06 UTC
[PROPOSAL] java gen: Uniform AsyncMethodCallback use in client and server
I'd like to propose a breaking change in both the java generated service
code and in supporting library code. This change would change
parametrization and use of clients to be symmetric with servers in the java
async stack.
Today the situation is as described in comments in
https://issues.apache.org/jira/browse/THRIFT-3112.
Although you might expect an async client call that returns an int to look
like:
`client.add(int first, int second, new AsyncMethodCallback<Integer>()
{...});`
It in fact looks like:
`client.add(int first, int second, new AsyncMethodCallback<add_call>()
{...});`
Where `add_call` is a type with an `int getResult() throws ...` method the
client must call in the `AsyncMethodCallback.onComplete` implementation to
retrieve the result of the async call, or else its remote error.
This has been the case since initial commit in 2010, but there are several
shortcomings, chief among these being:
1. The parametrization is highly un-expected. Its neither the expected
return type (like the server parametrization) nor a type at the same
abstraction level as the decalring AsyncIface - its a type encapsulated by
the AsyncClient _implementation_.
2. Because of the coupling to implementation in 1, we are not as free to
modify the library implementation or generated code as we might be in a
world where the AsyncIface was composed purely of standard value types
(structs, ints, etc).
It seems to me it would be wonderful to take our pre 1.0.0 status to
correct this confusing and leaky API.
I'm interested in what folks think.
Re: [PROPOSAL] java gen: Uniform AsyncMethodCallback use in client
and server
Posted by John Sirois <js...@apache.org>.
On Wed, Feb 10, 2016 at 3:31 AM, Aki Sukegawa <ns...@apache.org> wrote:
> +1
>
> The current signature is really awkward and the new design makes sense to
> me.
> It wouldn't be pleasant for every existing user but this clutter is worth
> removing IMO.
> The patch looks great too, BTW.
>
Thanks for the feedback Aki.
Since the patch is now green under the Jenkins Thrift-precommit job andyou
have offered positive feedback, I'll proceed with the patch and request
final review and merging by the committers.
> On Tue, Feb 9, 2016 at 5:10 AM John Sirois <js...@apache.org> wrote:
>
> > On Mon, Feb 8, 2016 at 12:40 PM, John Sirois <js...@apache.org> wrote:
> >
> > > I'd like to propose a breaking change in both the java generated
> service
> > > code and in supporting library code. This change would change
> > > parametrization and use of clients to be symmetric with servers in the
> > java
> > > async stack.
> > >
> > > Today the situation is as described in comments in
> > > https://issues.apache.org/jira/browse/THRIFT-3112.
> > > Although you might expect an async client call that returns an int to
> > look
> > > like:
> > > `client.add(int first, int second, new AsyncMethodCallback<Integer>()
> > > {...});`
> > >
> > > It in fact looks like:
> > > `client.add(int first, int second, new
> AsyncMethodCallback<add_call>()
> > > {...});`
> > >
> > > Where `add_call` is a type with an `int getResult() throws ...` method
> > the
> > > client must call in the `AsyncMethodCallback.onComplete` implementation
> > to
> > > retrieve the result of the async call, or else its remote error.
> > >
> > > This has been the case since initial commit in 2010, but there are
> > several
> > > shortcomings, chief among these being:
> > > 1. The parametrization is highly un-expected. Its neither the expected
> > > return type (like the server parametrization) nor a type at the same
> > > abstraction level as the decalring AsyncIface - its a type encapsulated
> > by
> > > the AsyncClient _implementation_.
> > > 2. Because of the coupling to implementation in 1, we are not as free
> to
> > > modify the library implementation or generated code as we might be in a
> > > world where the AsyncIface was composed purely of standard value types
> > > (structs, ints, etc).
> > >
> > > It seems to me it would be wonderful to take our pre 1.0.0 status to
> > > correct this confusing and leaky API.
> > >
> > > I'm interested in what folks think.
> > >
> >
> > To help make this proposal more concrete, I have a pull request with the
> > fully functioning diff with new tests here:
> > https://github.com/apache/thrift/pull/840
> >
>
Re: [PROPOSAL] java gen: Uniform AsyncMethodCallback use in client
and server
Posted by Aki Sukegawa <ns...@apache.org>.
+1
The current signature is really awkward and the new design makes sense to
me.
It wouldn't be pleasant for every existing user but this clutter is worth
removing IMO.
The patch looks great too, BTW.
On Tue, Feb 9, 2016 at 5:10 AM John Sirois <js...@apache.org> wrote:
> On Mon, Feb 8, 2016 at 12:40 PM, John Sirois <js...@apache.org> wrote:
>
> > I'd like to propose a breaking change in both the java generated service
> > code and in supporting library code. This change would change
> > parametrization and use of clients to be symmetric with servers in the
> java
> > async stack.
> >
> > Today the situation is as described in comments in
> > https://issues.apache.org/jira/browse/THRIFT-3112.
> > Although you might expect an async client call that returns an int to
> look
> > like:
> > `client.add(int first, int second, new AsyncMethodCallback<Integer>()
> > {...});`
> >
> > It in fact looks like:
> > `client.add(int first, int second, new AsyncMethodCallback<add_call>()
> > {...});`
> >
> > Where `add_call` is a type with an `int getResult() throws ...` method
> the
> > client must call in the `AsyncMethodCallback.onComplete` implementation
> to
> > retrieve the result of the async call, or else its remote error.
> >
> > This has been the case since initial commit in 2010, but there are
> several
> > shortcomings, chief among these being:
> > 1. The parametrization is highly un-expected. Its neither the expected
> > return type (like the server parametrization) nor a type at the same
> > abstraction level as the decalring AsyncIface - its a type encapsulated
> by
> > the AsyncClient _implementation_.
> > 2. Because of the coupling to implementation in 1, we are not as free to
> > modify the library implementation or generated code as we might be in a
> > world where the AsyncIface was composed purely of standard value types
> > (structs, ints, etc).
> >
> > It seems to me it would be wonderful to take our pre 1.0.0 status to
> > correct this confusing and leaky API.
> >
> > I'm interested in what folks think.
> >
>
> To help make this proposal more concrete, I have a pull request with the
> fully functioning diff with new tests here:
> https://github.com/apache/thrift/pull/840
>
Re: [PROPOSAL] java gen: Uniform AsyncMethodCallback use in client
and server
Posted by John Sirois <js...@apache.org>.
On Mon, Feb 8, 2016 at 12:40 PM, John Sirois <js...@apache.org> wrote:
> I'd like to propose a breaking change in both the java generated service
> code and in supporting library code. This change would change
> parametrization and use of clients to be symmetric with servers in the java
> async stack.
>
> Today the situation is as described in comments in
> https://issues.apache.org/jira/browse/THRIFT-3112.
> Although you might expect an async client call that returns an int to look
> like:
> `client.add(int first, int second, new AsyncMethodCallback<Integer>()
> {...});`
>
> It in fact looks like:
> `client.add(int first, int second, new AsyncMethodCallback<add_call>()
> {...});`
>
> Where `add_call` is a type with an `int getResult() throws ...` method the
> client must call in the `AsyncMethodCallback.onComplete` implementation to
> retrieve the result of the async call, or else its remote error.
>
> This has been the case since initial commit in 2010, but there are several
> shortcomings, chief among these being:
> 1. The parametrization is highly un-expected. Its neither the expected
> return type (like the server parametrization) nor a type at the same
> abstraction level as the decalring AsyncIface - its a type encapsulated by
> the AsyncClient _implementation_.
> 2. Because of the coupling to implementation in 1, we are not as free to
> modify the library implementation or generated code as we might be in a
> world where the AsyncIface was composed purely of standard value types
> (structs, ints, etc).
>
> It seems to me it would be wonderful to take our pre 1.0.0 status to
> correct this confusing and leaky API.
>
> I'm interested in what folks think.
>
To help make this proposal more concrete, I have a pull request with the
fully functioning diff with new tests here:
https://github.com/apache/thrift/pull/840