Replace Merge and MergeNamed with merge()
#238
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We already have 2 ways to join multiple receivers, `Merge` and `select`. `MergeNamed` is a mixture of both, so it is redundant. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
This is a more convenient way to use `Merge`, users normally only care about merging some channels, so having a class just makes the syntax more weird for most use cases that will be `async for msg in merge(...)`. `Merge` is not exposed to the user any more. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
This is just to make it more consistent with the internal attribute. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
Since `merge()` uses the `_Merge` class, when printing what's returned by `merge()` the string representation of `_Merge` is used. This is not very useful since it doesn't show the name that the user wrote in the code. This commit adds a `name` argument to `_Merge` and uses it to create the string representation of the receiver. If no name is provided, the class name is used as before. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
github-actions
bot
added
part:docs
llucax
added
the
scope:breaking-change
daniel-zullo-frequenz
previously approved these changes
Nov 10, 2023
shsms
reviewed
Nov 14, 2023
|
Thanks @llucax, I have no further comments, will approve once you fixup. |
Calling `merge()` with no receivers makes returns a stopped receiver,
which can be unexpected if it was done accidentally, as the user will
probably expect the merge receiver to block while receiving at least
once.
By raising an exception we make sure that users express their intentions
explicitly instead, having to write something like:
```python
receivers: list[Receiver[T]]
...
if receivers:
async for m in merge(*receivers):
...
```
In this case it is clear the user is having the no-receivers case into
account and will handle it properly.
Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
Apply suggestions made by @daniel-zullo-frequenz. Co-authored-by: daniel-zullo-frequenz <120166726+daniel-zullo-frequenz@users.noreply.github.com> Signed-off-by: Leandro Lucarella <luca@llucax.com>
In case `merge()` is called with one receiver there is no need to create a `_Merge` object, we can just return the provided receiver instead. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
Just to be consistent to how docstrings are normally written and organized. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
The current tests for merge are really integration tests, as it uses a channel, so we better rename it and mark it as such, as a failure in this test could be actually a failure in `Anycast` instead. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
|
Rebased to apply the fixups and enabled auto-merge. |
shsms
approved these changes
Nov 16, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
MergeNamedmerge()wrapper function that replacesMergeMergeto_Mergeto make it private more explicitly_Mergevariableargstoreceiversmerge()is called with less than two receiversFixes #236, fixes #237.