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.

Sign up for GitHub

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

Jump to bottom

Mono.fromCompletionStage behaves differently from the documentation #3235

Closed
benwiles1 opened this issue Oct 17, 2022 · 4 comments
Closed

Mono.fromCompletionStage behaves differently from the documentation #3235

benwiles1 opened this issue Oct 17, 2022 · 4 comments
Labels
type/documentation A documentation update warn/behavior-change Breaking change of publicly advertised behavior
Milestone
3.4.25

Comments

@benwiles1
Copy link

benwiles1 commented Oct 17, 2022 •
edited

Documentation Issue

Mono.fromCompletionStage is documented to not cancel the future, but in pull request #3146 it was updated to do so.

Additional context

I was previously relying on the future not being cancelled, what is the recommended way to to get that behavior back?
@reactorbot reactorbot added the ❓need-triage This issue needs triage, hasn't been looked at by a team member yet label Oct 17, 2022
@benwiles1 benwiles1 changed the title Mono.fromCompletableFuture behaves differently from the documentation Mono.fromCompletionStage behaves differently from the documentation Oct 17, 2022
@OlegDokuka OlegDokuka added type/documentation A documentation update warn/behavior-change Breaking change of publicly advertised behavior and removed ❓need-triage This issue needs triage, hasn't been looked at by a team member yet labels Oct 18, 2022
@OlegDokuka OlegDokuka added this to the 3.4.25 milestone Oct 19, 2022
@OlegDokuka
Copy link
Contributor

OlegDokuka commented Oct 19, 2022 •
edited

Indeed it is an unexpected behavior change. Will roll it back and rework it to keep old expectations as well as to have configurable cancellation supported for the CompletableFuture

@OlegDokuka OlegDokuka mentioned this issue Oct 26, 2022
Merged
@rstoyanchev
Copy link
Contributor

@benwiles1, could elaborate on your use case? Granted it is a regression, and that some control should be provided, it's not intuitive that cancel should not be passed on, and it would be useful to have a concrete scenario in mind.

@benwiles1
Copy link
Author

benwiles1 commented Oct 26, 2022 •
edited

I was reusing the same CompletableFuture that was saved to a field and just wrapping it in a Mono on the fly as needed. I have realized that it probably makes more sense to instead just wrap the future into a mono once and .cache() it. (and it probably could be implemented from the ground up without using futures, I just haven't had the time to go back and improve it since getting more experience with this library)

I don't actually have a problem with the future being canceled, mostly just annoyed that the documentation wasn't updated. and it was convenient at the time when I was learning the library.

@xinlian12 xinlian12 mentioned this issue Nov 2, 2022
Merged
@simonbasle simonbasle mentioned this issue Nov 4, 2022
Merged
@simonbasle
Copy link
Member

Fixed by #3272.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
type/documentation A documentation update warn/behavior-change Breaking change of publicly advertised behavior
Projects
None yet
Milestone
3.4.25
Development

No branches or pull requests
5 participants
@rstoyanchev @OlegDokuka @simonbasle @benwiles1 @reactorbot