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

Add Mono.fromCompletionStage overrides suppressing cancellation #3266

Closed
wants to merge 1 commit into from

Conversation

chemicL
Copy link
Member

@chemicL chemicL commented Nov 2, 2022

Recently, Mono.fromFuture operators were enhanced to support suppresion of subscription cancellation being propagated to the source Future. In case of CompletionStage, this overrides can be also useful, since cancelling the source was a behaviour change and users might need to default to the old behaviour.

This is a follow-up from #3146 and #3252

Recently, Mono::fromFuture operators were enhanced to support suppresion
of subscription cancellation being propagated to the source Future.
In case of CompletionStage, this overrides can be also useful, since
cancelling the source was a behaviour change and users might need to
default to the old behaviour.
@chemicL chemicL added the type/enhancement A general enhancement label Nov 2, 2022
@chemicL chemicL added this to the 3.4.25 milestone Nov 2, 2022
@chemicL chemicL self-assigned this Nov 2, 2022
@chemicL chemicL requested a review from a team as a code owner November 2, 2022 14:43
@chemicL chemicL changed the title Add Mono::fromCompletionStage overrides suppressing cancellation Add Mono.fromCompletionStage overrides suppressing cancellation Nov 2, 2022
* <p>
* <img class="marble" src="doc-files/marbles/fromFuture.svg" alt="">
* <p>
* Note, use {@link #fromFuture(CompletableFuture, boolean)} with {@code
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* Note, use {@link #fromFuture(CompletableFuture, boolean)} with {@code
* Note, use {@link #fromCompletionStage(CompletableFuture, boolean)} with {@code

* <p>
* <img class="marble" src="doc-files/marbles/fromFutureSupplier.svg" alt="">
* <p>
* Note, use {@link #fromFuture(CompletableFuture, boolean)} with {@code
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* Note, use {@link #fromFuture(CompletableFuture, boolean)} with {@code
* Note, use {@link #fromCompletionStage(CompletableFuture, boolean)} with {@code

Copy link
Member

@simonbasle simonbasle left a comment

Choose a reason for hiding this comment

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

as discussed I don't think this is the right course of action since that parameter doesn't make sense for all possible inputs (namely, CompletionStage instances that don't implement Future)

@chemicL
Copy link
Member Author

chemicL commented Nov 3, 2022

Closing. It does make sense when the CompletionStage also implements Future, but does not support conversion to CompletableFuture (in which case the fromFuture operator could be used). But we can come back to this PR and improve the javadoc, which is inaccurate at the moment, if users run into this problem and want to disable cancellation propagation.

@chemicL chemicL closed this Nov 3, 2022
@chemicL chemicL deleted the suppress-cancellation-completionstage branch November 30, 2023 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement A general enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants