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

FluxWindowWhile drains the inner window if it is cancelled #2651

Merged
merged 2 commits into from Mar 22, 2021
Merged

FluxWindowWhile drains the inner window if it is cancelled #2651

merged 2 commits into from Mar 22, 2021

Conversation

bruto1
Copy link
Contributor

@bruto1 bruto1 commented Mar 15, 2021

the other window operators seem to have their request in order

@bruto1 bruto1 requested a review from a team as a code owner March 15, 2021 06:38
@pivotal-issuemaster
Copy link

@bruto1 Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

1 similar comment
@pivotal-issuemaster
Copy link

@bruto1 Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@pivotal-issuemaster
Copy link

@bruto1 Thank you for signing the Contributor License Agreement!

@simonbasle simonbasle self-assigned this Mar 22, 2021
@simonbasle
Copy link
Member

hi @bruto1 thanks for the PR.
I think this is acceptable indeed, however given the workaround I wonder about the extent to which this can be considered a bug vs changing the behavior. In other words, should we target both 3.3.x and 3.4.x with this change, or only the later? 🤔

@bruto1
Copy link
Contributor Author

bruto1 commented Mar 22, 2021 •
edited

that's up to you, @simonbasle
I'm more interested in getting this fixed in 3.4 so I can throw out my slapdash "fix" with unbounded request

on the other hand, do you really think anyone relies on windowWhile/Until not discarding elements and stalling under specific circumstances?

@simonbasle
Copy link
Member

on the other hand, do you really think anyone relies on windowWhile/Until not discarding elements and stalling under specific circumstances?

fair enough 😄

@simonbasle simonbasle linked an issue Mar 22, 2021 that may be closed by this pull request
windowUntilX runs out of request/prefetch if windows are not fully drained #2585
Closed
@simonbasle simonbasle changed the title fixes #2585: insufficient request to upstream upon window cancellation in windowWhile/Until operators FluxWindowWhile drains the inner window if it is cancelled Mar 22, 2021
@simonbasle simonbasle merged commit 069a796 into reactor:3.3.x Mar 22, 2021
@reactorbot
Copy link

@simonbasle this PR seems to have been merged on a maintenance branch, please ensure the change is merge-forwarded to intermediate maintenance branches and up to master 🙇

simonbasle added a commit that referenced this pull request Mar 22, 2021
@simonbasle
Merge #2651 into 3.4.5
79bcd6f 
Conflicts:
 - Flux uses bold paragraph for discard support rather than javadoc tag
 - Use of Sinks in tests + contextWrite rather than subscriberContext()
@simonbasle simonbasle added this to the 3.4.5 milestone Mar 22, 2021
@bruto1 bruto1 deleted the 2585-windowPrematureCancel branch March 23, 2021 02:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@simonbasle simonbasle simonbasle approved these changes

Assignees

@simonbasle simonbasle

Labels
None yet
Projects
None yet
Milestone
3.4.5
Development

Successfully merging this pull request may close these issues.

windowUntilX runs out of request/prefetch if windows are not fully drained
4 participants
@bruto1 @pivotal-issuemaster @simonbasle @reactorbot