-
Notifications
You must be signed in to change notification settings - Fork 186
move Java 9 Flow support into main Source and Sink classes #2460
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
| override def createPublisher(elements: Long): Publisher[Int] = { | ||
| val sourceViaJavaFlowPublisher: JavaFlow.Publisher[Int] = Source(iterable(elements)) | ||
| .runWith(JavaFlowSupport.Sink.asPublisher(fanout = false)) | ||
| .runWith(Sink.asJavaPublisher(fanout = false)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice, much clear
|
I wonder if we shouldn't go a step further, and instead of adding a |
There is #1977. I would prefer to treat that as a separate issue. In that issue, @He-Pin points out that some libs still prefer reactivestreams lib (due to inertia or continuing desire to support Java 8). I have little issue with supporting both. It just seems like we shouldn't treat the Java Flow API as a 2nd class citizen. |
Much agreed. Perhaps we could make |
Another alternative would be to have static methods i.e. That way the method name is the same, but you import a different object depending on if you want Java's flow or Reactive Streams |
|
@mdedetrich @raboof Is it just the asJavaSubscriber and asJavaProducer that is causing concerns? If I separate out those changes but keep the moves of the fromPublisher and fromSubscriber methods, is that ok? Those methods can be overloaded because they don't hit issues with generics and type erasure. |
For me the design is cleaner to have If the design is breaking with Pekko 1.3.x you can create alias methods in Pekko 1.3.x that point to the new design in 2.0.0 and deprecate the older ones (Pekko 1.3.x hasn't been released yet). |
|
I am focusing on 2.x changes. I'm not going to rush a change to suit the 1.3.0 timeline. Pekko 2.x is built with Java 17 making it easier to support and test Java Flow API integration. |
|
We already have JavaFlowSupport so I guess the feedback is that some people prefer that to us moving the code to Source and Sink directly. The question then arises about us deprecating the org.reactivestreams code in Source and Sink and creating a ReactiveStreamsSupport class that mirrors the JavaFlowSupport. A more radical reworking sees us deprecating JavaFlowSupport and creating a new class that has similar utility functions but possibly with less verbose naming and then to mirror this new class to create an org.reactivestreams equivalent. |
I am aware, the point is not about rushing changes to suite a 1.3.x changeline but rather creating an ideal API design given that we have both reactive streams and java 9 flow If that API design requires changing the current reactive-streams interopt then its possible to create the necessary changes (i.e. aliases/deprecations) in 1.3.0 branch, thats all |
If the classes/datastructure didn't have the exact same name for both reactive streams and java 9 flow I would have also preferred this option, but in both cases they are called You could avoid this on the scaladsl side since you can use implicit classes/extension methods and you only import the support that you can are about (i.e. import Another disadvantage of inlining the source is that its not modular, given that already have java 9 flow/reactive streams flow we could have yet another streaming interface in the future which we might want to implement. Hence another argument for having separate objects/classes with static methods that provide the conversion methods. |
Uh oh!
There was an error while loading. Please reload this page.