-
Notifications
You must be signed in to change notification settings - Fork 196
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
Avoid Dropping Upstream Items in mergeSource
#513
Conversation
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.
Seems like a good change. Can you add a comment to the changelog and do a minor version bump? An added test case would be great too, but not a blocker.
I added both a changelog entry and a test case. |
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.
Thanks!
There's a build error in the tests due to missing the |
I pushed right as you wrote that, let's hope it works now. ;) |
LGTM, thanks! |
## 1.3.6 * Avoid dropping upstream items in `mergeSource` [#513](snoyberg/conduit#513)
From what I understand,
mergeSource
willawait
an item from upstream, and if successful,await
an item fromsrc0
. If there is no item insrc0
, the conduit terminates. This means that the item that was already taken from upstream is now lost (note the missing item4
in the output):This PR adds the already consumed item back using
leftover
, leading to the expected result:Disclaimer: I am pretty new to Conduit, so it is possible that I am either misunderstanding something or that there is a good reason for the current behavior of
mergeSource
, in which case, feel free to correct me.