Skip to content
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

Introduce an alternative to Processor-with-upstream case #3042

Merged
merged 9 commits into from
May 23, 2022

Conversation

simonbasle
Copy link
Member

@simonbasle simonbasle commented May 13, 2022

This introduces a migration path for a subset of Processors (namely the
EmitterProcessor) before they get removed from the public API in 3.5.0.

The ManyUpstreamAdapter is a Processor-adjactent API that doesn't expose
the CoreSubscriber API but rather separates concerns:

  • use of the Publisher aspect via asFlux() (similar to Sinks.Many)
  • subscription to an upstream via subscribeTo(Publisher)

This way, the underlying CoreSubscriber nature is never revealed to the user
which avoids the trouble with non-serialized access.

Emitter-like variants are exposed via Sinks.unsafe().manyToUpstream()
as .onBackpressureBuffer(...) variants mirroring the ones in MulticastSpec.

questions

  • should equivalents of ReplayProcessor also be added? (IIRC it also kind
    of supports the "subscribe-to-upstream-and-respect-downstream-backpressure"
    use case) => for now, focusing on EmitterProcessor
  • can this be introduced in 3.5.0 directly instead?

@simonbasle simonbasle added type/enhancement A general enhancement status/need-decision This needs a decision from the team for/team-attention This issue needs team attention or action labels May 13, 2022
@simonbasle simonbasle added this to the 3.4.19 milestone May 13, 2022
Copy link
Contributor

@OlegDokuka OlegDokuka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@simonbasle simonbasle marked this pull request as ready for review May 18, 2022 10:23
@simonbasle simonbasle requested a review from a team as a code owner May 18, 2022 10:23
Copy link
Contributor

@OlegDokuka OlegDokuka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good overall with one comment

@simonbasle simonbasle merged commit b07f391 into 3.4.x May 23, 2022
@simonbasle simonbasle deleted the replacementApiForEmitterProcessor branch May 23, 2022 15:39
simonbasle added a commit that referenced this pull request May 23, 2022
OlegDokuka added a commit that referenced this pull request May 31, 2022
simonbasle added a commit that referenced this pull request Jun 7, 2022
This commit reworks #3042 and reverts the changes to RootSpec returning
methods. These changes were binary incompatible with the previous
release of reactor, despite an API-compatible change.

What is reverted:
 - the `UnsafeRootSpec` and intermediate specs are removed
 - `Sinks.unsafe()` is back to returning `RootSpec`

What is kept:
 - `RootSpec` is dedicated to `Sinks.unsafe()`
 - `Sinks.manyWithUpstream` interface is still introduced

What is modified:
 - `RootSpec` receives an additional `manyWithUpstream()` method which
 is the one exposing `Sinks.manyWithUpstream` implementations
simonbasle added a commit that referenced this pull request Jun 7, 2022
This commit reworks #3042 and reverts the changes to RootSpec returning
methods. These changes were binary incompatible with the previous
release of reactor, despite an API-compatible change.

What is reverted:
 - the `UnsafeRootSpec` and intermediate specs are removed
 - `Sinks.unsafe()` is back to returning `RootSpec`

What is kept:
 - `RootSpec` is dedicated to `Sinks.unsafe()`
 - `Sinks.manyWithUpstream` interface is still introduced

What is modified:
 - `RootSpec` receives an additional `manyWithUpstream()` method which
 is the one exposing `Sinks.manyWithUpstream` implementations
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
for/team-attention This issue needs team attention or action status/need-decision This needs a decision from the team type/enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants