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 notes about merge, multiMap, and multiple input channels #3037

Merged
merged 3 commits into from
Aug 17, 2022

Conversation

bentsherman
Copy link
Member

From some other discussions (#2113 #2682), it looks like some users like to use the following pattern:

  1. start with a channel of tuples or maps
  2. use multiMap to split tuple/map into individual channels
  3. invoke process with the channels from (2) as inputs
  4. re-combine any output channels from (3) with merge

I think this pattern is used in order to avoid having long tuples for process input/output, but actually I think tuples are the best practice here, and this multiMap+merge pattern should be discouraged, because there is a risk of items not being re-combined correctly. I think it happens to work for now but order preservation is not guaranteed in principle.

I think the broader issue is that users would like to have better support for optional inputs/outputs and named inputs (i.e. map input), and this pattern is a sort of workaround for that. But for now I think we should reinforce the best practice in the docs, so in this PR I added some clarifications around use of merge, multiMap, and multiple input channels.

@bentsherman bentsherman requested a review from pditommaso July 15, 2022 18:46
@bentsherman bentsherman changed the title docs: add notes about merge, multiMap, and multiple input channels Add notes about merge, multiMap, and multiple input channels Jul 15, 2022
Signed-off-by: Ben Sherman <[email protected]>
@bentsherman
Copy link
Member Author

Added a deprecation warning to merge because it is actually deprecated in DSL2 according to the code. Apparently it's only allowed in DSL1.

Copy link
Member

@pditommaso pditommaso left a comment

Choose a reason for hiding this comment

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

Nice

@pditommaso pditommaso merged commit a1be895 into master Aug 17, 2022
@pditommaso pditommaso deleted the ben/docs-merge-multimap-warning branch August 17, 2022 10:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants