Streaming: Make events optional #154
Conversation
github-actions
bot
added
part:tests
There was a problem hiding this comment.
Pull Request Overview
This PR makes events optional in the streaming functionality, allowing receivers to be created with or without stream status events. Key changes include:
- Updating test cases to parameterize the inclusion of events.
- Adding overloads and modifying the new_receiver implementation to support an include_events flag.
- Adjusting channel handling and closure logic to separately manage data and event channels.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/streaming/test_grpc_stream_broadcaster.py | Updated tests to handle optional stream events via parameterization. |
| src/frequenz/client/base/streaming.py | Modified the GrpcStreamBroadcaster API to support an include_events flag with corresponding channel and overload changes. |
| try: | ||
| await self._task | ||
| except asyncio.CancelledError: | ||
| pass |
There was a problem hiding this comment.
The logic for closing the data and event channels is duplicated in both the stop and _run methods. Consider refactoring this duplicate code into a helper function to reduce repetition and improve maintainability.
| pass | |
| pass | |
| await self._close_channels() | |
| """Close the data and event channels.""" |
Signed-off-by: Mathias L. Baumann <[email protected]>
| maxsize: int = 50, | ||
| warn_on_overflow: bool = True, | ||
| *, |
There was a problem hiding this comment.
Not really part of this PR, but since you are adding the * here it makes me think that we should probably put all other arguments as keyword-only, as it can't be really inferred what they are by the type.
- **Make all parameters of streamers `new_receiver` named** #154 (comment)
2 participants
No description provided.