The main concern for me at the moment is that as far as
RxInvokerProvider is concerned, the current proposal is about allowing
RxInvoker actions be implemented in terms of running a SyncInvoker
action on a CompletionStage/etc default (or application provided) exec
service thread over a *blocking* HttpUrlConnection.
I'm still trying to figure out, if an option existed for the
implementers to let CompletionStage pool threads interact with a
non-blocking async transport, would it be a more effective option. Right
now, 2.1-m02 for this option too.
Any thoughts on the possible advantages/disadvantages of either approach
? I think it is important we understand it
Sergey
On 17/01/17 15:49, Santiago Pericasgeertsen wrote:
> Ondrej,
>
> It’s an interesting observation, and certainly makes sense on paper.
> But the are details that make this difficult to realize in practice.
> RxInvoker<T> is a parametric type whose get() method returns T;
> AsyncInvoker’s get() returns Future<Response>.
>
> It may be easier to support AsyncInvoker in the new scheme that the
> other way around because of backward compatibility.
>
> — Santiago
>
>> On Jan 17, 2017, at 9:11 AM, Ondrej Mihályi <ondrej.mihalyi_at_gmail.com
>> <mailto:ondrej.mihalyi_at_gmail.com>> wrote:
>>
>> I agree with Sergey here that Rx is mostly a sugar around the
>> AsyncInvoker's methods that accept InvocationCallback.
>>
>> In fact, for a while I was thinking: Why we just don't add new
>> methods to the AsyncInvoker instead of creating all the fuss around
>> RxInvoker? If we blend it with the RxCompletionStage which I
>> suggested in another thread, it would be simple, easy to understand
>> and use. I find it hard to justify having another rx() method next to
>> the async() method, unless the AsyncInvoker cannot be extended due to
>> breaking backwards compatibility (I don't think so).
>>
>> In the end, reactive is just async with callback, very often
>> providing something like CompletionStage to avoid callback hell. On
>> top of it, the callback can be executed multiple times for stream of
>> events, but I don't see a reason why a REST client call would emit
>> more events for a single request.
>>
>> I understand that Jersey decided to provide a separate RxInvoker to
>> avoid interfering with the current standard AsyncInvoker interface.
>> But within a standard, we can afford to add methods to AsyncInvoker
>> interface, or not?
>>
>> Just a food for thought, to realize if we didn't stray too much from
>> the original intentions...
>>
>> Ondrej
>>
>> 2017-01-17 14:24 GMT+01:00 Sergey Beryozkin <sberyozkin_at_talend.com
>> <mailto:sberyozkin_at_talend.com>>:
>>
>> Sure, I see how it works, as far as having the sync call staged
>> as part of the reactive chain.
>> To be honest this is not how I thought about Rx initially.
>>
>> For example, the Jersey blog I referred to earlier (and not only
>> the Jersey one) have referred to Rx as a remedy against the
>> InvocationCallback
>> noise which is obviously seen only with .async(). That is why I
>> started implementing with the initial assumption that for users
>> Rx is = an easier Async, easier composable, no callbacks, etc.
>>
>> While you are now saying that well, it is all about running the
>> sync action on the executor thread.
>> Isn't it what Async effectively also about ?
>>
>> I'd fine with having it all simplified, the simpler the better,
>> and settle down at SyncInvoker providing a method call action for
>> the CompletionStage/etc.
>>
>> But I wonder won't we miss something if we do it ? Is running
>> Async calls totally orthogonal to the idea of Rx ? I.e, will our
>> users ask us one day, why exactly I can't stage the async calls
>> as part of my RX flows ?
>>
>>
>>
>> Cheers, Sergey
>>
>> On 17/01/17 11:20, Pavel Bucek wrote:
>>>
>>> Just a thought:
>>>
>>>
>>> Why would we need to have access to Invocation.Builder#async()
>>> for creating CompletionStage (or others?). Isnt that (for now)
>>> about running the (synchronous!) action on an executor service
>>> (container or explicitly specified)? Async would create just
>>> another thread from that thread.
>>>
>>>
>>> Does it make sense? :-)
>>>
>>>
>>> I was looking at following code when I was thinking about the
>>> issue you raised:
>>>
>>>
>>> https://github.com/jersey/jersey/blob/2.26-b01/core-client/src/main/java/org/glassfish/jersey/client/JerseyCompletionStageRxInvoker.java#L65
>>> <https://github.com/jersey/jersey/blob/2.26-b01/core-client/src/main/java/org/glassfish/jersey/client/JerseyCompletionStageRxInvoker.java#L65>
>>>
>>>
>>> Please let me know what you think about that and if you reached
>>> same / different conclusion.
>>>
>>>
>>> Thanks!
>>>
>>> Pavel
>>>
>>>
>>> On 16/01/2017 19:10, Sergey Beryozkin wrote:
>>>> Hi Pavel
>>>>
>>>> I'm not sure at the moment.
>>>> In CXF, say, a CompletionStage invoker, only works on the async
>>>> HTTP transport. So we do supply a Supplier and the async thread
>>>> will wait inside this Supplier till the result is avail from
>>>> the async transport thread.
>>>> Expecting RxInvoker implementations will work with the
>>>> SyncInvoker alone to support them may not always work...
>>>> I glanced earlier at JerseyCompletionStageRxInvoker and looks
>>>> like an HTTP invocation over the sync transport is .supplyAsync-ed.
>>>> But as I said we do it over the async transport only - may be
>>>> it is not needed, but may be it is ?
>>>> I wonder if some new abstraction may need to be introduced.
>>>>
>>>> Cheers, Sergey
>>>>
>>>>
>>>> On 16/01/17 17:27, Pavel Bucek wrote:
>>>>>
>>>>> Hi Sergey,
>>>>>
>>>>> good catch!
>>>>>
>>>>> Would it be enough to change the param to SyncInvoker?
>>>>> (Invocation.Builder already extends that, so it would be very
>>>>> simple change).
>>>>>
>>>>> Thanks,
>>>>> Pavel
>>>>>
>>>>>
>>>>> On 16/01/2017 17:55, Sergey Beryozkin wrote:
>>>>>> Hi Pavel
>>>>>>
>>>>>> Looks like Invocation.Builder.rx() methods which accept
>>>>>> RxInvokerProvider are visible to these RxInvokerProviders.
>>>>>>
>>>>>> This is problematic. I see Jersey RxInvokerProviders delegate
>>>>>> back to Invocation.Builder so I can appreciate why
>>>>>> Invocation.Builder is passed on,
>>>>>>
>>>>>> but it just does not look right to me that rx() and
>>>>>> similarly, async() bridges, are still visible to the providers.
>>>>>>
>>>>>> Cheers, Sergey
>>>>>>
>>>>>> On 13/01/17 20:36, Pavel Bucek wrote:
>>>>>>>
>>>>>>> Dear experts,
>>>>>>>
>>>>>>> please review following wiki and APIs:
>>>>>>>
>>>>>>> https://java.net/projects/jax-rs-spec/pages/RxClient
>>>>>>> <https://java.net/projects/jax-rs-spec/pages/RxClient>
>>>>>>>
>>>>>>> All added classes related to Reactive Client APIs are linked
>>>>>>> from that page, but let me allow a short recap.
>>>>>>>
>>>>>>> JAX-RS Client is being extended by the ability to provide a
>>>>>>> way how to process responses in reactive fashion. The change
>>>>>>> consists of:
>>>>>>>
>>>>>>> - adding rx(...) methods to Invocation.Builder
>>>>>>> - defining RxInvoker
>>>>>>> - allowing users to extend this API by providing
>>>>>>> RxInvokerProvider
>>>>>>>
>>>>>>> Specification will mandate implementation for
>>>>>>> CompletionStage from Java SE 8.
>>>>>>>
>>>>>>> Client code examples:
>>>>>>>
>>>>>>> - basic use
>>>>>>>
>>>>>>> CompletionStage<List<String>> cs =
>>>>>>> client.target("remote/forecast/{destination}")
>>>>>>> .resolveTemplate("destination","mars")
>>>>>>> .request()
>>>>>>> .header("Rx-User","Java8")
>>>>>>> .rx()// gets CompletionStageRxInvoker .get(new GenericType<List<String>>() {
>>>>>>> });
>>>>>>>
>>>>>>> cs.thenAccept(System.out::println);
>>>>>>>
>>>>>>> - using custom RxInvokerFactory (this is little artificial,
>>>>>>> since the factory just returns CompletionStageRxInvoker, but
>>>>>>> support for Observable from RxJava or ListenableFuture from
>>>>>>> Guava can be done in the exact same manner)
>>>>>>>
>>>>>>> CompletionStage<List<String>> cs =
>>>>>>> client.target("remote/forecast/{destination}")
>>>>>>> .resolveTemplate("destination","mars")
>>>>>>> .request()
>>>>>>> .header("Rx-User","Java8")
>>>>>>> .rx(CompletionStageRxInvokerProvider.class)
>>>>>>> .get(new GenericType<List<String>>() {
>>>>>>> });
>>>>>>>
>>>>>>> cs.thenAccept(System.out::println);
>>>>>>>
>>>>>>> Source links:
>>>>>>>
>>>>>>> -
>>>>>>> https://github.com/jax-rs/api/blob/2.1-m02/jaxrs-api/src/main/java/javax/ws/rs/client/Invocation.java#L298
>>>>>>> <https://github.com/jax-rs/api/blob/2.1-m02/jaxrs-api/src/main/java/javax/ws/rs/client/Invocation.java#L298>
>>>>>>>
>>>>>>> jax-rs/api
>>>>>>> <https://github.com/jax-rs/api/blob/2.1-m02/jaxrs-api/src/main/java/javax/ws/rs/client/Invocation.java#L298>
>>>>>>> github.com <http://github.com/>
>>>>>>> api - JAX-RS API Source Code
>>>>>>>
>>>>>>> -
>>>>>>> https://github.com/jax-rs/api/blob/2.1-m02/jaxrs-api/src/main/java/javax/ws/rs/client/RxInvoker.java
>>>>>>> <https://github.com/jax-rs/api/blob/2.1-m02/jaxrs-api/src/main/java/javax/ws/rs/client/RxInvoker.java>
>>>>>>> -
>>>>>>> https://github.com/jax-rs/api/blob/2.1-m02/jaxrs-api/src/main/java/javax/ws/rs/client/RxInvokerProvider.java
>>>>>>> <https://github.com/jax-rs/api/blob/2.1-m02/jaxrs-api/src/main/java/javax/ws/rs/client/RxInvokerProvider.java>
>>>>>>>
>>>>>>> Examples & tests:
>>>>>>>
>>>>>>> -
>>>>>>> https://github.com/jax-rs/api/blob/2.1-m02/jaxrs-api/src/test/java/javax/ws/rs/core/RxClientTest.java
>>>>>>> <https://github.com/jax-rs/api/blob/2.1-m02/jaxrs-api/src/test/java/javax/ws/rs/core/RxClientTest.java>
>>>>>>> -
>>>>>>> https://github.com/jersey/jersey/blob/2.x/core-client/src/test/java/org/glassfish/jersey/client/ClientRxTest.java#L86
>>>>>>> <https://github.com/jersey/jersey/blob/2.x/core-client/src/test/java/org/glassfish/jersey/client/ClientRxTest.java#L86>
>>>>>>>
>>>>>>> The last link is to the Jersey repository. Jersey version
>>>>>>> 2.26 will be JAX-RS 2.1 RI and branch 2.x is where the
>>>>>>> development will happen. Jersey 2.26-b01 (which is being
>>>>>>> released right now) implements all rx(...) methods; feel
>>>>>>> free to test/evaluate it there.
>>>>>>>
>>>>>>> Looking forward to your feedback!
>>>>>>>
>>>>>>> Thanks and regards, Pavel
>>>>>>>