Add metrics package compatible with v1alpha8#128
Add metrics package compatible with v1alpha8#128llucax merged 7 commits intofrequenz-floss:v0.x.xfrom
metrics package compatible with v1alpha8#128Conversation
Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
We will start to expose conversion functions, for which we will always use a sub-module called `proto` for these conversion functions, so we we do the same with `enum_proto`. The old version is kept as deprecated. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
These functions are copied from `frequenz-client-base` and renamed to match the naming convention in this project. The functions in `frequenz-client-base` will be eventually deprecated. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
There was a problem hiding this comment.
Pull request overview
This PR introduces a new metrics package compatible with the v1alpha8 API version, along with a new proto package for common protobuf conversion utilities. The changes deprecate the old enum_proto module in favor of the new proto.enum_from_proto function and improve deprecation messages across the codebase to include full module paths.
Key changes:
- Adds new
frequenz.client.common.protopackage withenum_from_proto,datetime_from_proto, anddatetime_to_protoutilities - Introduces
frequenz.client.common.metricspackage with comprehensive metric types (Metric,MetricSample,AggregatedMetricValue,Bounds, etc.) compatible with v1alpha8 - Deprecates
frequenz.client.common.enum_protomodule with improved deprecation messages that include full module paths - Updates all existing deprecation messages to include full module paths for better clarity
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
pyproject.toml |
Adds protobuf and hypothesis dependencies with types-protobuf for mypy |
src/frequenz/client/common/proto/__init__.py |
New package exporting protobuf conversion utilities |
src/frequenz/client/common/proto/_enum.py |
New module with enum_from_proto function moved from enum_proto |
src/frequenz/client/common/proto/_timestamp.py |
New module with datetime conversion utilities |
src/frequenz/client/common/metrics/__init__.py |
New package exporting metric-related types |
src/frequenz/client/common/metrics/_metric.py |
Defines Metric enum with all supported metric types |
src/frequenz/client/common/metrics/_bounds.py |
Defines Bounds dataclass for metric constraints |
src/frequenz/client/common/metrics/_sample.py |
Defines MetricSample, AggregatedMetricValue, and related types |
src/frequenz/client/common/metrics/proto/__init__.py |
Exports protobuf conversion functions for metrics |
src/frequenz/client/common/metrics/proto/_bounds.py |
Implements bounds_from_proto conversion |
src/frequenz/client/common/metrics/proto/_sample.py |
Implements metric sample protobuf conversions with issue tracking |
src/frequenz/client/common/enum_proto.py |
Deprecates enum_from_proto with message pointing to new location |
src/frequenz/client/common/pagination/__init__.py |
Updates deprecation messages to include full module paths |
src/frequenz/client/common/microgrid/components/__init__.py |
Updates deprecation messages for from_proto methods |
src/frequenz/client/common/microgrid/electrical_components/__init__.py |
Updates deprecation messages for from_proto methods |
src/frequenz/client/common/metric/__init__.py |
Updates deprecation message for from_proto method |
tests/test_streaming.py |
Updates import to use new proto.enum_from_proto |
tests/test_enum_proto.py |
Simplifies tests to only check deprecation warnings |
tests/test_client_common.py |
Adds deprecation warning checks to existing tests |
tests/proto/test_enum.py |
New comprehensive tests for enum_from_proto |
tests/proto/test_datetime.py |
New tests for datetime conversion using hypothesis |
tests/metrics/test_bounds.py |
New tests for Bounds class |
tests/metrics/test_sample.py |
New tests for MetricSample and AggregatedMetricValue |
tests/metrics/proto/test_bounds.py |
New tests for bounds protobuf conversion |
tests/metrics/proto/test_sample.py |
New tests for metric sample protobuf conversion |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
1a6ad99 to
77dfd1d
Compare
4d37299 to
d25a06e
Compare
|
Will update with some better test coverage. |
d25a06e to
9882428
Compare
9882428 to
19f9523
Compare
|
Updated release notes to make them ready for a release. |
|
|
||
|
|
||
| def bounds_from_proto(message: bounds_pb2.Bounds) -> Bounds: # noqa: DOC502 | ||
| """Create a `Bounds` from a protobuf message. |
There was a problem hiding this comment.
... Bounds object from ... ?
| major_issues: list[str], | ||
| minor_issues: list[str], # pylint: disable=unused-argument | ||
| ) -> Bounds | None: # noqa: DOC502 | ||
| """Create a `Bounds` from a protobuf message. |
| """The alternating current electric potential difference between phase 3 and phase 1.""" | ||
|
|
||
| AC_CURRENT = metrics_pb2.METRIC_AC_CURRENT | ||
| """The alternating current current.""" |
There was a problem hiding this comment.
This reads weird. I suggest to replace all "alternating current" strings with AC. If you think this needs to be defined (tbh I don't) you could do so in the beginning of the class doc.
There was a problem hiding this comment.
Will change, I thought these came from the API definition but it looks like it doesn't, not sure if it was in the previous versions or if it was generated by AI but I definitely didn't write that :D
There was a problem hiding this comment.
I also replaced all "direct current" with "DC".
| """The metric is unspecified (this should not be used).""" | ||
|
|
||
| DC_VOLTAGE = metrics_pb2.METRIC_DC_VOLTAGE | ||
| """The direct current voltage.""" |
There was a problem hiding this comment.
We should state the units for all of the metrics.
There was a problem hiding this comment.
I would first fix it here:
And then replicate in this repo. We probably need to create an issue in this repo too so we don't forget.
There was a problem hiding this comment.
OK, I added the units in the issue, and also a few more I thought it was safe to assume, but there are still some I'm not sure about.
19f9523 to
b54bd26
Compare
|
Updated. |
b54bd26 to
a0f3349
Compare
The old `metric` package (not the singular) is based on the old API v0.5. Since the package name didn't match the API naming scheme, we add a new package with the right name to keep backwards compatibility. These classes are based on the microgrid API client v0.18: https://github.com/frequenz-floss/frequenz-client-microgrid-python/tree/v0.18.0/src/frequenz/client/microgrid/metrics Only some minor changes were applied to make it closer to the API names, for example, `MetricSample.sampled_at` was renamed to `.sample_time` to match the new name. Also the `MetricConnection` and `MetricConnectionCategory` classes were added. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>
a0f3349 to
9121ffc
Compare
This PR introduces a new
metricspackage compatible with the v1alpha8 API version, along with a newprotopackage for common protobuf conversion utilities. The changes deprecate the oldenum_protomodule in favor of the newproto.enum_from_protofunction and improve deprecation messages across the codebase to include full module paths.Key changes:
frequenz.client.common.protopackage withenum_from_proto,datetime_from_proto, anddatetime_to_protoutilitiesfrequenz.client.common.metricspackage with comprehensive metric types (Metric,MetricSample,AggregatedMetricValue,Bounds, etc.) compatible with v1alpha8frequenz.client.common.enum_protomodule with improved deprecation messages that include full module paths