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 wiretap/wiretapContext to FlowWithContext/SourceWithContext #1446

Conversation

mdedetrich
Copy link
Contributor

Analogous to #1443 but with wireTap/wireTapContext

@mdedetrich mdedetrich force-pushed the add-wireTap-wireTapContext-to-Flow-Source-with-context branch 2 times, most recently from 2429d43 to b7cf181 Compare August 23, 2024 14:31
@pjfanning pjfanning added this to the 1.1.0 milestone Aug 23, 2024
@mdedetrich mdedetrich force-pushed the add-wireTap-wireTapContext-to-Flow-Source-with-context branch 3 times, most recently from 4615c7a to 8a9cd77 Compare August 24, 2024 11:26
@mdedetrich mdedetrich force-pushed the add-wireTap-wireTapContext-to-Flow-Source-with-context branch from 8a9cd77 to fd88c93 Compare August 24, 2024 12:43
@mdedetrich
Copy link
Contributor Author

So the tests in the PR are now working, I ended up having to use atLeastOneElementOf since the feature of wireTap is that it will drop elements if the downstream consumer is not ready (rather than back pressuring which is what alsoTo does).

In other words its possible that in certain test runs one of the elements may be missing, which is intentional behaviour of wireTap.

@mdedetrich mdedetrich force-pushed the add-wireTap-wireTapContext-to-Flow-Source-with-context branch from fd88c93 to 9948552 Compare August 24, 2024 18:24
@mdedetrich
Copy link
Contributor Author

mdedetrich commented Aug 26, 2024

@pjfanning Do you mind reviewing this PR? You have reviewed the previous version and its the last thing from my side for the next RC which you wanted to plan for Thursday.

@mdedetrich mdedetrich force-pushed the add-wireTap-wireTapContext-to-Flow-Source-with-context branch from 9948552 to 1d42eff Compare August 27, 2024 08:17
@mdedetrich mdedetrich requested a review from pjfanning August 27, 2024 08:17
Copy link
Contributor

@pjfanning pjfanning left a comment

Choose a reason for hiding this comment

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

lgtm

@mdedetrich mdedetrich merged commit f548ea5 into apache:main Aug 27, 2024
19 checks passed
@mdedetrich mdedetrich deleted the add-wireTap-wireTapContext-to-Flow-Source-with-context branch August 27, 2024 09:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants