Re: Reactive Streams dependency in Project Reactor module

[Sergey Beryozkin]

Hi John

I think I indeed sent some confusing messages yesterday :-)

OK, what I meant was that the module had the original code to do with 
supporting the client only Rx invocations (such a client code can be 
server scoped as in your example), and the server only invocations (Json 
subcriber).


When I moved some of the server specific code into a shared base module 
I thought, after the 1st iteration, where only the JSON subscriber was 
shared, that it was OK to make that module optional - as it was of no 
use in the client-only scope and was not strictly needed for the server 
too, but during the 2nd/3rd iteration, when I pushed some more common 
code there affecting the JAXRSInvoker extensions, I should've removed 
the optional dep...


Perhaps making that dependency required is the simplest way forward

Sergey

On 04/02/18 19:22, John D. Ament wrote:

Well, now that I understand that it was meant specifically for client only
(its kind of odd, because JsonStreamingAsyncSubscriber is really for
subscribers, which is more on the server produced response).

What if we just had distinct modules for reactive-client and
reactive-server?

But either way, I'm not sure I follow your thoughts yet, since my use case
is just taking an existing Flux/Mono and piping it to a AsyncResponse
(nothing to do with client).

Granted, by doing that, I'm relying on internal CXF code, but it works.

John

On Sun, Feb 4, 2018 at 2:04 PM Sergey Beryozkin 
wrote:


The same though applies to the client code - it makes no sense on the
server side, so may be it is just simpler to make that dep non-optional
for the consistency purpose, up to you guys...

Sergey
On 04/02/18 18:57, Sergey Beryozkin wrote:

You've already concluded it is a bug...

I recall now, I made it optional because that code makes no sense on the
client side only, while the reactive streams api is also pulled from the
reactor dep...

Cheers, Sergey
On 04/02/18 18:12, Andriy Redko wrote:

Same conclusion, it shouldn't be optional/provided.
Thanks for spotting it.

Best Regards,
  Andriy Redko

JDA> That's what I'm asking basically.  If you look at
JDA>


https://github.com/apache/cxf/blob/master/rt/rs/extensions/reactor/pom.xml#L47-L49


JDA> I
JDA> don't believe it should be provided/optional.

JDA> On Sun, Feb 4, 2018 at 12:49 PM Sergey Beryozkin

JDA> wrote:


Why should be optional ?



Sergey
On 04/02/18 14:00, John D. Ament wrote:

Hi,

As far as I can tell, the dependency on reactive streams isn't
optional

in

the project reactor module.  I'm wondering, was this just a typo,
or am I
missing something?

John

Re: Reactive Streams dependency in Project Reactor module

[John D. Ament]

Well, now that I understand that it was meant specifically for client only
(its kind of odd, because JsonStreamingAsyncSubscriber is really for
subscribers, which is more on the server produced response).

What if we just had distinct modules for reactive-client and
reactive-server?

But either way, I'm not sure I follow your thoughts yet, since my use case
is just taking an existing Flux/Mono and piping it to a AsyncResponse
(nothing to do with client).

Granted, by doing that, I'm relying on internal CXF code, but it works.

John

On Sun, Feb 4, 2018 at 2:04 PM Sergey Beryozkin 
wrote:

> The same though applies to the client code - it makes no sense on the
> server side, so may be it is just simpler to make that dep non-optional
> for the consistency purpose, up to you guys...
>
> Sergey
> On 04/02/18 18:57, Sergey Beryozkin wrote:
> > You've already concluded it is a bug...
> >
> > I recall now, I made it optional because that code makes no sense on the
> > client side only, while the reactive streams api is also pulled from the
> > reactor dep...
> >
> > Cheers, Sergey
> > On 04/02/18 18:12, Andriy Redko wrote:
> >> Same conclusion, it shouldn't be optional/provided.
> >> Thanks for spotting it.
> >>
> >> Best Regards,
> >>  Andriy Redko
> >>
> >> JDA> That's what I'm asking basically.  If you look at
> >> JDA>
> >>
> https://github.com/apache/cxf/blob/master/rt/rs/extensions/reactor/pom.xml#L47-L49
> >>
> >> JDA> I
> >> JDA> don't believe it should be provided/optional.
> >>
> >> JDA> On Sun, Feb 4, 2018 at 12:49 PM Sergey Beryozkin
> >> 
> >> JDA> wrote:
> >>
>  Why should be optional ?
> >>
>  Sergey
>  On 04/02/18 14:00, John D. Ament wrote:
> > Hi,
> >
> > As far as I can tell, the dependency on reactive streams isn't
> > optional
>  in
> > the project reactor module.  I'm wondering, was this just a typo,
> > or am I
> > missing something?
> >
> > John
> >
> >>
> >>
> >>
> >
>
>

Re: Reactive Streams dependency in Project Reactor module

[Sergey Beryozkin]

The same though applies to the client code - it makes no sense on the 
server side, so may be it is just simpler to make that dep non-optional 
for the consistency purpose, up to you guys...


Sergey
On 04/02/18 18:57, Sergey Beryozkin wrote:

You've already concluded it is a bug...

I recall now, I made it optional because that code makes no sense on the 
client side only, while the reactive streams api is also pulled from the 
reactor dep...


Cheers, Sergey
On 04/02/18 18:12, Andriy Redko wrote:

Same conclusion, it shouldn't be optional/provided.
Thanks for spotting it.

Best Regards,
 Andriy Redko

JDA> That's what I'm asking basically.  If you look at
JDA> 
https://github.com/apache/cxf/blob/master/rt/rs/extensions/reactor/pom.xml#L47-L49 


JDA> I
JDA> don't believe it should be provided/optional.

JDA> On Sun, Feb 4, 2018 at 12:49 PM Sergey Beryozkin 


JDA> wrote:


Why should be optional ?



Sergey
On 04/02/18 14:00, John D. Ament wrote:

Hi,

As far as I can tell, the dependency on reactive streams isn't 
optional

in
the project reactor module.  I'm wondering, was this just a typo, 
or am I

missing something?

John

Re: Reactive Streams dependency in Project Reactor module

[Sergey Beryozkin]

You've already concluded it is a bug...

I recall now, I made it optional because that code makes no sense on the 
client side only, while the reactive streams api is also pulled from the 
reactor dep...


Cheers, Sergey
On 04/02/18 18:12, Andriy Redko wrote:

Same conclusion, it shouldn't be optional/provided.
Thanks for spotting it.

Best Regards,
 Andriy Redko

JDA> That's what I'm asking basically.  If you look at
JDA> 
https://github.com/apache/cxf/blob/master/rt/rs/extensions/reactor/pom.xml#L47-L49
JDA> I
JDA> don't believe it should be provided/optional.

JDA> On Sun, Feb 4, 2018 at 12:49 PM Sergey Beryozkin 
JDA> wrote:


Why should be optional ?



Sergey
On 04/02/18 14:00, John D. Ament wrote:

Hi,

As far as I can tell, the dependency on reactive streams isn't optional

in

the project reactor module.  I'm wondering, was this just a typo, or am I
missing something?

John

Re: Reactive Streams dependency in Project Reactor module

[Andriy Redko]

Same conclusion, it shouldn't be optional/provided.
Thanks for spotting it.

Best Regards,
Andriy Redko

JDA> That's what I'm asking basically.  If you look at
JDA> 
https://github.com/apache/cxf/blob/master/rt/rs/extensions/reactor/pom.xml#L47-L49
JDA> I
JDA> don't believe it should be provided/optional.

JDA> On Sun, Feb 4, 2018 at 12:49 PM Sergey Beryozkin 
JDA> wrote:

>> Why should be optional ?

>> Sergey
>> On 04/02/18 14:00, John D. Ament wrote:
>> > Hi,
>> >
>> > As far as I can tell, the dependency on reactive streams isn't optional
>> in
>> > the project reactor module.  I'm wondering, was this just a typo, or am I
>> > missing something?
>> >
>> > John
>> >

Re: Reactive Streams dependency in Project Reactor module

[John D. Ament]

That's what I'm asking basically.  If you look at
https://github.com/apache/cxf/blob/master/rt/rs/extensions/reactor/pom.xml#L47-L49
I
don't believe it should be provided/optional.

On Sun, Feb 4, 2018 at 12:49 PM Sergey Beryozkin 
wrote:

> Why should be optional ?
>
> Sergey
> On 04/02/18 14:00, John D. Ament wrote:
> > Hi,
> >
> > As far as I can tell, the dependency on reactive streams isn't optional
> in
> > the project reactor module.  I'm wondering, was this just a typo, or am I
> > missing something?
> >
> > John
> >
>
>