Skip to content

Conversation

@shsms
Copy link
Contributor

@shsms shsms commented Jun 10, 2025

Also remove unneecessary async declarations.

Copilot AI review requested due to automatic review settings June 10, 2025 09:25
@shsms shsms added cmd:skip-release-notes It is not necessary to update release notes for this PR and removed cmd:skip-release-notes It is not necessary to update release notes for this PR labels Jun 10, 2025
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This pull request refactors the streaming interface for reporting by reusing GrpcStreamBroadcasters for aggregated data streams and removing unnecessary async declarations. Key changes include updating method signatures to return Receivers instead of async generators, modifying docstrings to reflect the new return types, and introducing a new aggregated data streams dictionary keyed by metric parameters.

Comments suppressed due to low confidence (3)

src/frequenz/client/reporting/_client.py:512

  • Changing receive_aggregated_data from an async function to a synchronous one returning a Receiver is a significant interface update; ensure that all consumers of this API are updated accordingly to handle the new behavior.
def receive_aggregated_data(

src/frequenz/client/reporting/_client.py:120

  • [nitpick] Consider adding inline documentation for the tuple key structure used in _aggregated_data_streams to improve code clarity and maintainability.
self._aggregated_data_streams: dict[

src/frequenz/client/reporting/_client.py:308

  • Refactoring stream_method to return a casted AsyncIterable instead of using an async generator simplifies the implementation, but please verify that the stub method indeed returns an AsyncIterable to avoid runtime issues.
def stream_method() -> (

Also the note about some functions becoming non-async, is moved to the
`Upgrading` section, because it is a breaking change.

Signed-off-by: Sahas Subramanian <[email protected]>
@github-actions github-actions bot added the part:docs Affects the documentation label Jun 10, 2025
@shsms shsms enabled auto-merge June 10, 2025 09:36
@shsms shsms merged commit dd24055 into frequenz-floss:v0.x.x Jun 10, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

part:docs Affects the documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants