Skip to content

Conversation

@flora-hofmann-frequenz
Copy link
Collaborator

Think we forgot to take out 'async' when refactoring in #177

@github-actions github-actions bot added the part:docs Affects the documentation label Jun 9, 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 PR removes unneeded asynchronous behavior from sensor data reporting functions.

  • Removed the "async" keyword and corresponding "await" calls in two sensor data functions and the helper method.
  • Updated RELEASE_NOTES.md to document the removal of asynchronous behavior in sensor handling.

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/frequenz/client/reporting/_client.py Removed async declarations from some sensor data methods.
RELEASE_NOTES.md Updated release notes reflecting the async removal.
Comments suppressed due to low confidence (1)

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

  • The function receive_single_sensor_data is still declared as async despite calling a now synchronous helper. Consider removing the async keyword from its definition for consistency if no asynchronous operations remain.
receiver = self._receive_microgrid_sensors_data_batch(

@shsms
Copy link
Contributor

shsms commented Jun 10, 2025

Oops, looks like we need to remove it from the aggregated data method as well.

@flora-hofmann-frequenz
Copy link
Collaborator Author

Oops, looks like we need to remove it from the aggregated data method as well.

I would like to do that in a separate PR.

I would also have to touch _batch_unroll_receiver to add the Receiver for aggregated data, no? Or would I leave it as an iterator?

@shsms
Copy link
Contributor

shsms commented Jun 10, 2025

I would also have to touch _batch_unroll_receiver to add the Receiver for aggregated data, no? Or would I leave it as an iterator?

oh damn, I did that, but pushed to my branch and not to yours, even though I wrote in your PR that I had done it, so it didn't get merged. 🤦🏽

I'll make a PR with the remaining commits now.

@flora-hofmann-frequenz
Copy link
Collaborator Author

oh damn, I did that, but pushed to my branch and not to yours, even though I wrote in your PR that I had done it, so it didn't get merged. 🤦🏽

I'll make a PR with the remaining commits now.

Great, thanks a lot @shsms

@flora-hofmann-frequenz flora-hofmann-frequenz merged commit ad031dc 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