Skip to content

Conversation

@llucax
Copy link
Contributor

@llucax llucax commented Jun 10, 2025

This PR updates the microgrid client to use IDs from the frequenz-client-common package instead of defining them in the microgrid client itself. This change is part of a broader effort to centralize common functionality and types across different clients.

@Copilot Copilot AI review requested due to automatic review settings June 10, 2025 08:31
@llucax llucax requested review from a team as code owners June 10, 2025 08:31
@github-actions github-actions bot added part:docs Affects the documentation part:tests Affects the unit, integration and performance (benchmarks) tests part:tooling Affects the development tooling (CI, deployment, dependency management, etc.) part:client Affects the client code 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

Centralize ID types by importing them from the frequenz-client-common package instead of defining them locally in the microgrid client.

  • Replaced local SensorId, ComponentId, and MicrogridId definitions with imports from frequenz-client-common
  • Removed the id.py module and updated all code and tests accordingly
  • Added frequenz-client-common as a dependency in pyproject.toml and documented the change in RELEASE_NOTES.md

Reviewed Changes

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

Show a summary per file
File Description
tests/test_sensor_proto.py Swapped local SensorId import for frequenz.client.common
tests/test_sensor.py Swapped local SensorId import for frequenz.client.common
tests/test_metadata.py Swapped local MicrogridId import for frequenz.client.common
tests/test_id.py Swapped local MicrogridId, ComponentId, SensorId imports
tests/test_connection.py Swapped local ComponentId import for frequenz.client.common
tests/test_component_data.py Swapped local ComponentId import for frequenz.client.common
tests/test_component.py Swapped local ComponentId import for frequenz.client.common
tests/test_client.py Swapped local ID imports for frequenz.client.common
src/frequenz/client/microgrid/sensor.py Swapped local SensorId import for frequenz.client.common
src/frequenz/client/microgrid/_sensor_proto.py Swapped local SensorId import for frequenz.client.common
src/frequenz/client/microgrid/_metadata.py Swapped local MicrogridId import for frequenz.client.common
src/frequenz/client/microgrid/_connection.py Swapped local ComponentId import for frequenz.client.common
src/frequenz/client/microgrid/_component_data.py Swapped local ComponentId import for frequenz.client.common
src/frequenz/client/microgrid/_component.py Swapped local ComponentId import for frequenz.client.common
src/frequenz/client/microgrid/_client.py Swapped local ID imports for frequenz.client.common
src/frequenz/client/microgrid/id.py Removed local ID definitions
pyproject.toml Added frequenz-client-common dependency
RELEASE_NOTES.md Documented ID import changes

@llucax llucax self-assigned this Jun 10, 2025
@llucax llucax added this to the v0.9.0 milestone Jun 10, 2025
llucax added 3 commits June 10, 2025 14:27
This commit updates the microgrid client to use IDs from the
`frequenz-client-common` package instead of defining them in the
microgrid client itself. This change is part of a broader effort to
centralize common functionality and types across different clients.

We also bump some minimum dependency versions of some other dependencies
to be able to work with the new `frequenz-client-common` dependency.

Signed-off-by: Leandro Lucarella <[email protected]>
These are just warnings and we are already aware, and will upgrade as
needed when we are 2 majors behind.

Signed-off-by: Leandro Lucarella <[email protected]>
Signed-off-by: Leandro Lucarella <[email protected]>
@llucax llucax enabled auto-merge June 10, 2025 12:28
@llucax llucax requested a review from shsms June 10, 2025 13:31
@llucax llucax added this pull request to the merge queue Jun 10, 2025
Merged via the queue into frequenz-floss:v0.x.x with commit 11e4c5e Jun 10, 2025
5 checks passed
@llucax llucax deleted the common-ids branch June 10, 2025 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

part:client Affects the client code part:docs Affects the documentation part:tests Affects the unit, integration and performance (benchmarks) tests part:tooling Affects the development tooling (CI, deployment, dependency management, etc.)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants