-
Notifications
You must be signed in to change notification settings - Fork 5
Upgrade the client to v0.18.x (v0.8.0 common) #190
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this 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 upgrades the microgrid client to support API v0.18.0 (microgrid) and v0.8.0 (common). The upgrade primarily involves updating import paths, renaming symbols to match the new API structure, and adapting to behavioral changes in the API calls.
Key Changes:
- Updated API import paths from
v1tov1alpha18for microgrid API andv1alpha8for common API - Renamed component-related field names (e.g.,
component_id→electrical_component_id) - Changed power setting methods from unary-unary to unary-stream calls, returning only the first response
- Added deprecated aliases for renamed component categories and metrics to maintain backward compatibility
Reviewed Changes
Copilot reviewed 85 out of 85 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/frequenz/client/microgrid/_client.py |
Updated gRPC method calls and field names to match new API structure |
src/frequenz/client/microgrid/component/_category.py |
Added deprecated aliases for renamed categories (GRID → GRID_CONNECTION_POINT, VOLTAGE_TRANSFORMER → POWER_TRANSFORMER) |
src/frequenz/client/microgrid/metrics/_metric.py |
Added deprecated aliases for renamed metrics (AC_ACTIVE_POWER → AC_POWER_ACTIVE, etc.) |
tests/ |
Updated test files to use new API paths and field names |
pyproject.toml |
Updated dependencies to use new API versions and removed timezonefinder dependency |
Comments suppressed due to low confidence (1)
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
For this we also need to update the common API dependency to v0.8.0 and the client common dependency to v0.3.6. Signed-off-by: Leandro Lucarella <[email protected]>
This component was removed from the microgrid API v0.18. Signed-off-by: Leandro Lucarella <[email protected]>
These were removed from the microgrid API v0.18. Signed-off-by: Leandro Lucarella <[email protected]>
Signed-off-by: Leandro Lucarella <[email protected]>
Originally testing unary-stream calls assumed a `GrpcStreamBroadcaster` is used, which means a `Receiver` is returned synchronously. But not all unary-stream calls adopt this pattern, and some might be async, so returning a coroutine instead. With this commit we inspect the return value and if it is a coroutine, we await for it and assign the awaited value as the client result. Signed-off-by: Leandro Lucarella <[email protected]>
This commit updates the import and perform simple symbol renames, as well as adapting some structural changes. The interface is not affected, changes should be backwards compatible, although some minor behaviour changes might be present (for example responses of set_component_power_xxx()). The structural changes are: * set_component_power_active()/reactive() changed from unary-unary to unary-stream. For now we are only receiving the first response and returning that, but this will be improved in the future. * add_component_bounds() changed the Validity enum to have int values to cope with the more flexible API. In the future we'll add the possibility to pass arbitrary validities. * ComponentStateSample's errors and warnings are now wrapped in a message with multiple fields, for now we are only retrieving the code. In the future we'll wrap the new fields too. * MetricSample.connection is now also wrapped in a message with multiple fields. We are only getting the connection name for now, but we'll add the new information in the future too. * Some component categories were renamed, we added the new names but kept the old ones as deprecated. We needed to add the new names in this commit to avoid having to adapt the code too much, as the category name is tied to the category_specific_info.kind value, so we needed the new values. We need to disable the "unique" constraint for ComponentCategory temporarily to allow for the deprecated aliases. Other changes (and there are many pending) are left as future improvements. Some of these pending changes are: * Rename component -> electrical component (including IDs and fields) * Rename grid -> grid connection point * Rename voltage transformer -> power transformer * Rename relay -> breaker * Rename SOLAR -> PV * Rename source -> connection * Add new component categories, Metrics and other new enum fields * Deprecate UNDERVOLTAGE_SHUTDOWN and old Metric names * Rename category_speific_metadata -> category_specific_info * Rename sampled_at -> origin_time / sample_time Signed-off-by: Leandro Lucarella <[email protected]>
With this we can enable `@unique` again, as we can explicitly mark members as deprecated and deprecated members are not checked for duplicates. We also replace the internal uses of the deprecated names with the new names. Signed-off-by: Leandro Lucarella <[email protected]>
Signed-off-by: Leandro Lucarella <[email protected]>
Signed-off-by: Leandro Lucarella <[email protected]>
We still keep the old names but we mark them as deprecated. We need to disable the max line check in flake8 for the whole file because the symbol names are too long, and we can't use per-line overrides because this happens inside a docstring. Signed-off-by: Leandro Lucarella <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! Thanks for this :)
|
I ran some basic tests and it seems it is working as expected, so I will merge. |
This PR does a minimal upgrade to be able to use the microgrid API v0.18.0 and common API v0.8.0.
It mainly updates the import and perform simple symbol renames, as well as adapting some structural changes. The interface is not affected, changes should be backwards compatible, although some minor behavior changes might be present (for example responses of
set_component_power_xxx()).The structural changes are:
set_component_power_active()/reactive()changed from unary-unary to unary-stream. For now, we are only receiving the first response and returning that, but this will be improved in the future.add_component_bounds()changed the Validity enum to have int values to cope with the more flexible API. In the future we'll add the possibility to pass an arbitrary validity.ComponentStateSample's errors and warnings are now wrapped in a message with multiple fields, for now we are only retrieving the code. In the future we'll wrap the new fields too.MetricSample.connectionis now also wrapped in a message with multiple fields. We are only getting the connection name for now, but we'll add the new information in the future too.Some component categories were renamed, we added the new names but kept the old ones as deprecated. We needed to add the new names in this commit to avoid having to adapt the code too much, as the category name is tied to the
category_specific_info.kindvalue, so we needed the new values. We need to disable the "unique" constraint forComponentCategorytemporarily to allow for the deprecated aliases.Other changes (and there are many pending) are left as future improvements. Some of these pending changes are: