From 262bbe265450a4cb2d29baaa50bd6542a28bbce5 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Mon, 13 Jul 2026 11:51:56 +0000 Subject: [PATCH 01/12] Make `ElectricalComponent.model` required The `model` is a required field in the protobuf message so it is better to keep the representation consistent. An empty string is preserved as is now. Signed-off-by: Leandro Lucarella --- .../_electrical_component.py | 2 +- .../proto/v1alpha8/_electrical_component.py | 10 +++------- .../proto/v1alpha8/test_class.py | 1 + .../proto/v1alpha8/test_raw_storage.py | 3 +++ .../electrical_components/test_battery.py | 6 ++++++ .../test_electrical_component_base.py | 17 ++++++++++++++++- .../electrical_components/test_ev_charger.py | 6 ++++++ .../test_grid_connection_point.py | 3 +++ .../electrical_components/test_inverter.py | 6 ++++++ .../test_power_transformer.py | 1 + .../electrical_components/test_problematic.py | 5 +++++ .../test_simple_components.py | 1 + 12 files changed, 52 insertions(+), 9 deletions(-) diff --git a/src/frequenz/client/common/microgrid/electrical_components/_electrical_component.py b/src/frequenz/client/common/microgrid/electrical_components/_electrical_component.py index 1c06514f..31dba2ce 100644 --- a/src/frequenz/client/common/microgrid/electrical_components/_electrical_component.py +++ b/src/frequenz/client/common/microgrid/electrical_components/_electrical_component.py @@ -28,7 +28,7 @@ class ElectricalComponent: # pylint: disable=too-many-instance-attributes name: str """The name of this electrical component.""" - model: str | None = None + model: str """The model of this electrical component. This includes both the manufacturer and the model name. diff --git a/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component.py b/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component.py index 58375949..d1c555e1 100644 --- a/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component.py +++ b/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component.py @@ -882,8 +882,8 @@ class _ElectricalComponentBaseData(NamedTuple): name: str """The human-readable name of the electrical component.""" - model: str | None - """The optional model string of the electrical component.""" + model: str + """The model string of the electrical component.""" category: ElectricalComponentCategory | int """The category of the electrical component.""" @@ -929,10 +929,6 @@ def _electrical_component_base_from_proto_with_issues( component_id = ElectricalComponentId(message.id) microgrid_id = MicrogridId(message.microgrid_id) - model = message.model or None - if model is None: - minor_issues.append("model is empty") - provides_telemetry, accepts_control = _operational_mode_to_bools( message.operational_mode ) @@ -977,7 +973,7 @@ def _electrical_component_base_from_proto_with_issues( component_id, microgrid_id, message.name, - model, + message.model, category, lifetime, metric_config_bounds, diff --git a/tests/microgrid/electrical_components/proto/v1alpha8/test_class.py b/tests/microgrid/electrical_components/proto/v1alpha8/test_class.py index db57f252..9a96e127 100644 --- a/tests/microgrid/electrical_components/proto/v1alpha8/test_class.py +++ b/tests/microgrid/electrical_components/proto/v1alpha8/test_class.py @@ -180,6 +180,7 @@ "id": ElectricalComponentId(1), "microgrid_id": MicrogridId(1), "name": "", + "model": "Test Model", "_provides_telemetry": True, "_accepts_control": True, "_allow_construction": True, diff --git a/tests/microgrid/electrical_components/proto/v1alpha8/test_raw_storage.py b/tests/microgrid/electrical_components/proto/v1alpha8/test_raw_storage.py index 01627c17..8efcc6ac 100644 --- a/tests/microgrid/electrical_components/proto/v1alpha8/test_raw_storage.py +++ b/tests/microgrid/electrical_components/proto/v1alpha8/test_raw_storage.py @@ -62,6 +62,7 @@ def test_raw_type_preserved_for_unknown( id=default_component_base_data.component_id, microgrid_id=default_component_base_data.microgrid_id, name="", + model=default_component_base_data.model, type=999, _provides_telemetry=True, _accepts_control=True, @@ -94,6 +95,7 @@ def test_unrecognized_type_shows_in_repr( id=default_component_base_data.component_id, microgrid_id=default_component_base_data.microgrid_id, name="", + model=default_component_base_data.model, type=999, _provides_telemetry=True, _accepts_control=True, @@ -112,6 +114,7 @@ def _unrecognized_battery(battery_type: int) -> UnrecognizedBattery: id=default_component_base_data.component_id, microgrid_id=default_component_base_data.microgrid_id, name="", + model=default_component_base_data.model, type=battery_type, _provides_telemetry=True, _accepts_control=True, diff --git a/tests/microgrid/electrical_components/test_battery.py b/tests/microgrid/electrical_components/test_battery.py index 4e47d85d..6cca936b 100644 --- a/tests/microgrid/electrical_components/test_battery.py +++ b/tests/microgrid/electrical_components/test_battery.py @@ -38,6 +38,7 @@ def test_abstract_battery_cannot_be_instantiated( id=component_id, microgrid_id=microgrid_id, name="test_battery", + model="Test Model", _provides_telemetry=True, _accepts_control=True, ) @@ -58,6 +59,7 @@ def test_recognized_battery_types( id=component_id, microgrid_id=microgrid_id, name="test_battery", + model="Test Model", _provides_telemetry=True, _accepts_control=True, _allow_construction=True, @@ -76,6 +78,7 @@ def test_unrecognized_battery_type( id=component_id, microgrid_id=microgrid_id, name="unrecognized_battery", + model="Test Model", type=999, _provides_telemetry=True, _accepts_control=True, @@ -96,6 +99,7 @@ def test_unspecified_battery_is_problematic( id=component_id, microgrid_id=microgrid_id, name="", + model="Test Model", _provides_telemetry=True, _accepts_control=True, _allow_construction=True, @@ -113,6 +117,7 @@ def test_unrecognized_battery_is_problematic( id=component_id, microgrid_id=microgrid_id, name="", + model="Test Model", type=999, _provides_telemetry=True, _accepts_control=True, @@ -134,6 +139,7 @@ def test_recognized_battery_types_are_not_problematic( id=component_id, microgrid_id=microgrid_id, name="", + model="Test Model", _provides_telemetry=True, _accepts_control=True, _allow_construction=True, diff --git a/tests/microgrid/electrical_components/test_electrical_component_base.py b/tests/microgrid/electrical_components/test_electrical_component_base.py index d18c68c6..241289ff 100644 --- a/tests/microgrid/electrical_components/test_electrical_component_base.py +++ b/tests/microgrid/electrical_components/test_electrical_component_base.py @@ -34,6 +34,7 @@ def test_base_creation_fails() -> None: id=ElectricalComponentId(1), microgrid_id=MicrogridId(1), name="", + model="Test Model", _provides_telemetry=True, _accepts_control=True, ) @@ -46,6 +47,7 @@ def test_direct_construction_without_flag_raises() -> None: id=ElectricalComponentId(1), microgrid_id=MicrogridId(2), name="", + model="Test Model", _provides_telemetry=True, _accepts_control=True, ) @@ -57,13 +59,14 @@ def test_creation_with_defaults() -> None: id=ElectricalComponentId(1), microgrid_id=MicrogridId(2), name="", + model="Test Model", _provides_telemetry=True, _accepts_control=True, _allow_construction=True, ) assert component.name == "" - assert component.model is None + assert component.model == "Test Model" assert component.operational_lifetime == Lifetime() assert component.metric_config_bounds == {} assert component.category_specific_metadata == {} @@ -99,6 +102,7 @@ def test_accessors_return_values_when_set() -> None: id=ElectricalComponentId(1), microgrid_id=MicrogridId(2), name="", + model="Test Model", _provides_telemetry=True, _accepts_control=False, _allow_construction=True, @@ -114,6 +118,7 @@ def test_accessors_raise_when_unspecified() -> None: id=ElectricalComponentId(1), microgrid_id=MicrogridId(2), name="", + model="Test Model", _provides_telemetry=0, _accepts_control=0, _allow_construction=True, @@ -131,6 +136,7 @@ def test_accessors_raise_when_unrecognized() -> None: id=ElectricalComponentId(1), microgrid_id=MicrogridId(2), name="", + model="Test Model", _provides_telemetry=999, _accepts_control=999, _allow_construction=True, @@ -158,6 +164,7 @@ def test_str(name: str, expected_str: str) -> None: id=ElectricalComponentId(1), microgrid_id=MicrogridId(2), name=name, + model="Test Model", _provides_telemetry=True, _accepts_control=True, _allow_construction=True, @@ -177,6 +184,7 @@ def test_operational_at(is_operational: bool) -> None: id=ElectricalComponentId(1), microgrid_id=MicrogridId(1), name="", + model="Test Model", operational_lifetime=mock_lifetime, _provides_telemetry=True, _accepts_control=True, @@ -203,6 +211,7 @@ def test_is_operational_now(mock_datetime: Mock) -> None: id=ElectricalComponentId(1), microgrid_id=MicrogridId(1), name="", + model="Test Model", operational_lifetime=mock_lifetime, _provides_telemetry=True, _accepts_control=True, @@ -218,6 +227,7 @@ def test_is_operational_now(mock_datetime: Mock) -> None: id=ElectricalComponentId(1), microgrid_id=MicrogridId(1), name="test", + model="Test Model", metric_config_bounds={Metric.AC_POWER_ACTIVE: Bounds(lower=-100.0, upper=100.0)}, category_specific_metadata={"key": "value"}, _provides_telemetry=True, @@ -229,6 +239,7 @@ def test_is_operational_now(mock_datetime: Mock) -> None: id=COMPONENT.id, microgrid_id=COMPONENT.microgrid_id, name=COMPONENT.name, + model=COMPONENT.model, metric_config_bounds={Metric.AC_POWER_ACTIVE: Bounds(lower=-200.0, upper=200.0)}, category_specific_metadata={"different": "metadata"}, _provides_telemetry=True, @@ -240,6 +251,7 @@ def test_is_operational_now(mock_datetime: Mock) -> None: id=COMPONENT.id, microgrid_id=COMPONENT.microgrid_id, name="different", + model=COMPONENT.model, metric_config_bounds=COMPONENT.metric_config_bounds, category_specific_metadata=COMPONENT.category_specific_metadata, _provides_telemetry=True, @@ -251,6 +263,7 @@ def test_is_operational_now(mock_datetime: Mock) -> None: id=ElectricalComponentId(2), microgrid_id=COMPONENT.microgrid_id, name=COMPONENT.name, + model=COMPONENT.model, metric_config_bounds=COMPONENT.metric_config_bounds, category_specific_metadata=COMPONENT.category_specific_metadata, _provides_telemetry=True, @@ -262,6 +275,7 @@ def test_is_operational_now(mock_datetime: Mock) -> None: id=COMPONENT.id, microgrid_id=MicrogridId(2), name=COMPONENT.name, + model=COMPONENT.model, metric_config_bounds=COMPONENT.metric_config_bounds, category_specific_metadata=COMPONENT.category_specific_metadata, _provides_telemetry=True, @@ -273,6 +287,7 @@ def test_is_operational_now(mock_datetime: Mock) -> None: id=ElectricalComponentId(2), microgrid_id=MicrogridId(2), name=COMPONENT.name, + model=COMPONENT.model, metric_config_bounds=COMPONENT.metric_config_bounds, category_specific_metadata=COMPONENT.category_specific_metadata, _provides_telemetry=True, diff --git a/tests/microgrid/electrical_components/test_ev_charger.py b/tests/microgrid/electrical_components/test_ev_charger.py index 83568c2a..b1a50e1a 100644 --- a/tests/microgrid/electrical_components/test_ev_charger.py +++ b/tests/microgrid/electrical_components/test_ev_charger.py @@ -39,6 +39,7 @@ def test_abstract_ev_charger_cannot_be_instantiated( id=component_id, microgrid_id=microgrid_id, name="test_charger", + model="Test Model", _provides_telemetry=True, _accepts_control=True, ) @@ -59,6 +60,7 @@ def test_recognized_ev_charger_types( id=component_id, microgrid_id=microgrid_id, name="test_charger", + model="Test Model", _provides_telemetry=True, _accepts_control=True, _allow_construction=True, @@ -77,6 +79,7 @@ def test_unrecognized_ev_charger_type( id=component_id, microgrid_id=microgrid_id, name="unrecognized_charger", + model="Test Model", type=999, _provides_telemetry=True, _accepts_control=True, @@ -97,6 +100,7 @@ def test_unspecified_ev_charger_is_problematic( id=component_id, microgrid_id=microgrid_id, name="", + model="Test Model", _provides_telemetry=True, _accepts_control=True, _allow_construction=True, @@ -114,6 +118,7 @@ def test_unrecognized_ev_charger_is_problematic( id=component_id, microgrid_id=microgrid_id, name="", + model="Test Model", type=999, _provides_telemetry=True, _accepts_control=True, @@ -135,6 +140,7 @@ def test_recognized_ev_charger_types_are_not_problematic( id=component_id, microgrid_id=microgrid_id, name="", + model="Test Model", _provides_telemetry=True, _accepts_control=True, _allow_construction=True, diff --git a/tests/microgrid/electrical_components/test_grid_connection_point.py b/tests/microgrid/electrical_components/test_grid_connection_point.py index 2bbd70e5..144fd0b3 100644 --- a/tests/microgrid/electrical_components/test_grid_connection_point.py +++ b/tests/microgrid/electrical_components/test_grid_connection_point.py @@ -35,6 +35,7 @@ def test_creation_ok( id=component_id, microgrid_id=microgrid_id, name="test_grid_point", + model="Test Model", rated_fuse_current=rated_fuse_current, _provides_telemetry=True, _accepts_control=True, @@ -58,6 +59,7 @@ def test_creation_invalid_rated_fuse_current( id=component_id, microgrid_id=microgrid_id, name="test_grid_point", + model="Test Model", rated_fuse_current=-1, _provides_telemetry=True, _accepts_control=True, @@ -74,6 +76,7 @@ def test_creation_without_flag_raises( id=component_id, microgrid_id=microgrid_id, name="", + model="Test Model", rated_fuse_current=0, _provides_telemetry=True, _accepts_control=True, diff --git a/tests/microgrid/electrical_components/test_inverter.py b/tests/microgrid/electrical_components/test_inverter.py index 3ebfb3f5..92a1b261 100644 --- a/tests/microgrid/electrical_components/test_inverter.py +++ b/tests/microgrid/electrical_components/test_inverter.py @@ -39,6 +39,7 @@ def test_abstract_inverter_cannot_be_instantiated( id=component_id, microgrid_id=microgrid_id, name="test_inverter", + model="Test Model", _provides_telemetry=True, _accepts_control=True, ) @@ -59,6 +60,7 @@ def test_recognized_inverter_types( id=component_id, microgrid_id=microgrid_id, name="test_inverter", + model="Test Model", _provides_telemetry=True, _accepts_control=True, _allow_construction=True, @@ -77,6 +79,7 @@ def test_unrecognized_inverter_type( id=component_id, microgrid_id=microgrid_id, name="unrecognized_inverter", + model="Test Model", type=999, _provides_telemetry=True, _accepts_control=True, @@ -97,6 +100,7 @@ def test_unspecified_inverter_is_problematic( id=component_id, microgrid_id=microgrid_id, name="", + model="Test Model", _provides_telemetry=True, _accepts_control=True, _allow_construction=True, @@ -114,6 +118,7 @@ def test_unrecognized_inverter_is_problematic( id=component_id, microgrid_id=microgrid_id, name="", + model="Test Model", type=999, _provides_telemetry=True, _accepts_control=True, @@ -135,6 +140,7 @@ def test_recognized_inverter_types_are_not_problematic( id=component_id, microgrid_id=microgrid_id, name="", + model="Test Model", _provides_telemetry=True, _accepts_control=True, _allow_construction=True, diff --git a/tests/microgrid/electrical_components/test_power_transformer.py b/tests/microgrid/electrical_components/test_power_transformer.py index 2715787d..9f8eb7f0 100644 --- a/tests/microgrid/electrical_components/test_power_transformer.py +++ b/tests/microgrid/electrical_components/test_power_transformer.py @@ -38,6 +38,7 @@ def test_creation_ok( id=component_id, microgrid_id=microgrid_id, name="test_power_transformer", + model="Test Model", primary_voltage=primary, secondary_voltage=secondary, _provides_telemetry=True, diff --git a/tests/microgrid/electrical_components/test_problematic.py b/tests/microgrid/electrical_components/test_problematic.py index e0fbe40e..c814ff26 100644 --- a/tests/microgrid/electrical_components/test_problematic.py +++ b/tests/microgrid/electrical_components/test_problematic.py @@ -38,6 +38,7 @@ def test_abstract_problematic_electrical_component_cannot_be_instantiated( id=component_id, microgrid_id=microgrid_id, name="test_problematic", + model="Test Model", _provides_telemetry=True, _accepts_control=True, ) @@ -51,6 +52,7 @@ def test_unspecified_component( id=component_id, microgrid_id=microgrid_id, name="unspecified_component", + model="Test Model", _provides_telemetry=True, _accepts_control=True, _allow_construction=True, @@ -70,6 +72,7 @@ def test_mismatched_category_component_with_known_category( id=component_id, microgrid_id=microgrid_id, name="mismatched_battery", + model="Test Model", category=expected_category, _provides_telemetry=True, _accepts_control=True, @@ -91,6 +94,7 @@ def test_mismatched_category_component_with_unrecognized_category( id=component_id, microgrid_id=microgrid_id, name="mismatched_unrecognized", + model="Test Model", category=expected_category, _provides_telemetry=True, _accepts_control=True, @@ -111,6 +115,7 @@ def test_unrecognized_component_type( id=component_id, microgrid_id=microgrid_id, name="unrecognized_component", + model="Test Model", category=999, _provides_telemetry=True, _accepts_control=True, diff --git a/tests/microgrid/electrical_components/test_simple_components.py b/tests/microgrid/electrical_components/test_simple_components.py index 0e8be747..94c0dd1d 100644 --- a/tests/microgrid/electrical_components/test_simple_components.py +++ b/tests/microgrid/electrical_components/test_simple_components.py @@ -87,6 +87,7 @@ def test_init( id=component_id, microgrid_id=microgrid_id, name="test_component", + model="Test Model", _allow_construction=True, _provides_telemetry=True, _accepts_control=True, From 7d3d651b89ef45946125597ce71d86ff290a0f33 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Mon, 13 Jul 2026 12:36:36 +0000 Subject: [PATCH 02/12] Add `BaseLifetime` abstract base class This class will be used as the base for both valid and invalid lifetime classes. Signed-off-by: Leandro Lucarella --- src/frequenz/client/common/types/__init__.py | 3 ++- src/frequenz/client/common/types/_lifetime.py | 19 +++++++++++++++++++ tests/types/test_lifetime.py | 8 +++++++- 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/src/frequenz/client/common/types/__init__.py b/src/frequenz/client/common/types/__init__.py index 9ce60ed1..9311c724 100644 --- a/src/frequenz/client/common/types/__init__.py +++ b/src/frequenz/client/common/types/__init__.py @@ -3,7 +3,7 @@ """Common types.""" -from ._lifetime import Lifetime +from ._lifetime import BaseLifetime, Lifetime from ._location import ( InvalidCountryCode, InvalidCountryCodeError, @@ -15,6 +15,7 @@ ) __all__ = [ + "BaseLifetime", "InvalidCountryCode", "InvalidCountryCodeError", "InvalidLatitude", diff --git a/src/frequenz/client/common/types/_lifetime.py b/src/frequenz/client/common/types/_lifetime.py index 2ac1a8fc..27f2f15a 100644 --- a/src/frequenz/client/common/types/_lifetime.py +++ b/src/frequenz/client/common/types/_lifetime.py @@ -5,6 +5,25 @@ from dataclasses import dataclass from datetime import datetime, timezone +from typing import Any, Self + + +@dataclass(frozen=True, kw_only=True) +class BaseLifetime: + """A base class for all lifetimes.""" + + start_time: datetime | None = None + """The moment when the asset became operationally active.""" + + end_time: datetime | None = None + """The moment when the asset's operational activity ceased.""" + + # pylint: disable-next=unused-argument + def __new__(cls, *args: Any, **kwargs: Any) -> Self: + """Prevent instantiation of this class.""" + if cls is BaseLifetime: + raise TypeError(f"Cannot instantiate {cls.__name__} directly") + return super().__new__(cls) @dataclass(frozen=True, kw_only=True) diff --git a/tests/types/test_lifetime.py b/tests/types/test_lifetime.py index f0768872..dc5cdfd0 100644 --- a/tests/types/test_lifetime.py +++ b/tests/types/test_lifetime.py @@ -9,7 +9,7 @@ import pytest -from frequenz.client.common.types import Lifetime +from frequenz.client.common.types import BaseLifetime, Lifetime class _Time(Enum): @@ -97,6 +97,12 @@ def future(present: datetime) -> datetime: return present.replace(year=present.year + 1) +def test_base_lifetime_cannot_be_instantiated_directly() -> None: + """`BaseLifetime` refuses direct instantiation.""" + with pytest.raises(TypeError, match="Cannot instantiate BaseLifetime directly"): + BaseLifetime() + + @pytest.mark.parametrize( "case", [ From 10571ed4adb9a4b5ce2f3eb277217a2d8e785949 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Mon, 13 Jul 2026 12:37:40 +0000 Subject: [PATCH 03/12] Make `Lifetime` inherit from `BaseLifetime` `Lifetime`, the existing type, is retroactively made a subclass of `BaseLifetime`. Its field shape, construction API, validation, and operational checks are unchanged. Signed-off-by: Leandro Lucarella --- src/frequenz/client/common/types/_lifetime.py | 30 +++++++++---------- tests/types/test_lifetime.py | 5 ++++ 2 files changed, 19 insertions(+), 16 deletions(-) diff --git a/src/frequenz/client/common/types/_lifetime.py b/src/frequenz/client/common/types/_lifetime.py index 27f2f15a..b7419737 100644 --- a/src/frequenz/client/common/types/_lifetime.py +++ b/src/frequenz/client/common/types/_lifetime.py @@ -13,10 +13,17 @@ class BaseLifetime: """A base class for all lifetimes.""" start_time: datetime | None = None - """The moment when the asset became operationally active.""" + """The moment when the asset became operationally active. + + If `None`, the asset is considered to be active in any past moment previous to the + [`end_time`][..end_time]. + """ end_time: datetime | None = None - """The moment when the asset's operational activity ceased.""" + """The moment when the asset's operational activity ceased. + + If `None`, the asset is considered to be active with no plans to be deactivated. + """ # pylint: disable-next=unused-argument def __new__(cls, *args: Any, **kwargs: Any) -> Self: @@ -27,27 +34,18 @@ def __new__(cls, *args: Any, **kwargs: Any) -> Self: @dataclass(frozen=True, kw_only=True) -class Lifetime: +class Lifetime(BaseLifetime): """An active operational period of an asset. + When both [`start_time`][.start_time] and [`end_time`][.end_time] are + `None`, the lifetime is unbounded and the asset is considered operational + at every timestamp. + Warning: The [`end_time`][.end_time] timestamp indicates that the asset has been permanently removed from service. """ - start_time: datetime | None = None - """The moment when the asset became operationally active. - - If `None`, the asset is considered to be active in any past moment previous to the - [`end_time`][..end_time]. - """ - - end_time: datetime | None = None - """The moment when the asset's operational activity ceased. - - If `None`, the asset is considered to be active with no plans to be deactivated. - """ - def __post_init__(self) -> None: """Validate this lifetime.""" if ( diff --git a/tests/types/test_lifetime.py b/tests/types/test_lifetime.py index dc5cdfd0..c49d48d4 100644 --- a/tests/types/test_lifetime.py +++ b/tests/types/test_lifetime.py @@ -103,6 +103,11 @@ def test_base_lifetime_cannot_be_instantiated_directly() -> None: BaseLifetime() +def test_lifetime_is_base_lifetime_subclass() -> None: + """`Lifetime` is a subclass of `BaseLifetime`.""" + assert issubclass(Lifetime, BaseLifetime) + + @pytest.mark.parametrize( "case", [ From 1be62d010adb57686ea83c9797caaabf34f68ff4 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Mon, 13 Jul 2026 12:40:15 +0000 Subject: [PATCH 04/12] Add `InvalidLifetime` `InvalidLifetime` represents malformed wire data. It carries the same timestamps as `BaseLifetime` without enforcing ordering invariants, so callers can inspect what the server actually sent without using it for operational checks. Signed-off-by: Leandro Lucarella --- src/frequenz/client/common/types/__init__.py | 3 ++- src/frequenz/client/common/types/_lifetime.py | 25 ++++++++++++++++++- tests/types/test_lifetime.py | 17 ++++++++++++- 3 files changed, 42 insertions(+), 3 deletions(-) diff --git a/src/frequenz/client/common/types/__init__.py b/src/frequenz/client/common/types/__init__.py index 9311c724..12d6127c 100644 --- a/src/frequenz/client/common/types/__init__.py +++ b/src/frequenz/client/common/types/__init__.py @@ -3,7 +3,7 @@ """Common types.""" -from ._lifetime import BaseLifetime, Lifetime +from ._lifetime import BaseLifetime, InvalidLifetime, Lifetime from ._location import ( InvalidCountryCode, InvalidCountryCodeError, @@ -20,6 +20,7 @@ "InvalidCountryCodeError", "InvalidLatitude", "InvalidLatitudeError", + "InvalidLifetime", "InvalidLongitude", "InvalidLongitudeError", "Lifetime", diff --git a/src/frequenz/client/common/types/_lifetime.py b/src/frequenz/client/common/types/_lifetime.py index b7419737..a0c6b824 100644 --- a/src/frequenz/client/common/types/_lifetime.py +++ b/src/frequenz/client/common/types/_lifetime.py @@ -10,7 +10,12 @@ @dataclass(frozen=True, kw_only=True) class BaseLifetime: - """A base class for all lifetimes.""" + """A base class for well-formed and malformed operational lifetimes. + + This class cannot be instantiated directly. Use [`Lifetime`][..Lifetime] + for a valid period or [`InvalidLifetime`][..InvalidLifetime] to preserve + malformed wire data. + """ start_time: datetime | None = None """The moment when the asset became operationally active. @@ -44,6 +49,12 @@ class Lifetime(BaseLifetime): Warning: The [`end_time`][.end_time] timestamp indicates that the asset has been permanently removed from service. + + Note: + Raises a `ValueError` if [`start_time`][.start_time] is later than the + [`end_time`][.end_time] timestamp. Use + [`InvalidLifetime`][..InvalidLifetime] to represent malformed lifetime + data received from the wire. """ def __post_init__(self) -> None: @@ -73,3 +84,15 @@ def is_operational_at(self, timestamp: datetime) -> bool: def is_operational_now(self) -> bool: """Whether this lifetime is currently active.""" return self.is_operational_at(datetime.now(timezone.utc)) + + +@dataclass(frozen=True, kw_only=True) +class InvalidLifetime(BaseLifetime): + """An operational lifetime with malformed data received from the wire. + + This class preserves lifetime data that fails the invariants required for + a well-formed [`Lifetime`][..Lifetime], allowing callers to inspect the raw + timestamps without accidentally using them for operational checks. Use a + semantic accessor, such as `ElectricalComponent.get_operational_lifetime()`, + to receive a clear [`InvalidLifetimeError`][..InvalidLifetimeError]. + """ diff --git a/tests/types/test_lifetime.py b/tests/types/test_lifetime.py index c49d48d4..b5e988ce 100644 --- a/tests/types/test_lifetime.py +++ b/tests/types/test_lifetime.py @@ -9,7 +9,7 @@ import pytest -from frequenz.client.common.types import BaseLifetime, Lifetime +from frequenz.client.common.types import BaseLifetime, InvalidLifetime, Lifetime class _Time(Enum): @@ -108,6 +108,21 @@ def test_lifetime_is_base_lifetime_subclass() -> None: assert issubclass(Lifetime, BaseLifetime) +def test_invalid_lifetime_is_base_lifetime_subclass() -> None: + """`InvalidLifetime` is a subclass of `BaseLifetime`.""" + assert issubclass(InvalidLifetime, BaseLifetime) + + +def test_invalid_lifetime_accepts_invalid_range( + present: datetime, future: datetime +) -> None: + """`InvalidLifetime` preserves an end time before its start time.""" + lifetime = InvalidLifetime(start_time=future, end_time=present) + + assert lifetime.start_time is future + assert lifetime.end_time is present + + @pytest.mark.parametrize( "case", [ From 858cb31b24701a9ada147503ea2e65e9dc5afce7 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Mon, 13 Jul 2026 12:44:37 +0000 Subject: [PATCH 05/12] Add `InvalidLifetimeError` The dedicated `InvalidLifetimeError`, also a `ValueError` for convenience, carries the offending `InvalidLifetime` on `.lifetime`. Upcoming semantic accessors can therefore reject malformed lifetime data without preventing callers from inspecting the original timestamps. Signed-off-by: Leandro Lucarella --- src/frequenz/client/common/types/__init__.py | 8 ++- src/frequenz/client/common/types/_lifetime.py | 41 ++++++++++++++++ tests/types/test_lifetime.py | 49 ++++++++++++++++++- 3 files changed, 96 insertions(+), 2 deletions(-) diff --git a/src/frequenz/client/common/types/__init__.py b/src/frequenz/client/common/types/__init__.py index 12d6127c..18a52873 100644 --- a/src/frequenz/client/common/types/__init__.py +++ b/src/frequenz/client/common/types/__init__.py @@ -3,7 +3,12 @@ """Common types.""" -from ._lifetime import BaseLifetime, InvalidLifetime, Lifetime +from ._lifetime import ( + BaseLifetime, + InvalidLifetime, + InvalidLifetimeError, + Lifetime, +) from ._location import ( InvalidCountryCode, InvalidCountryCodeError, @@ -21,6 +26,7 @@ "InvalidLatitude", "InvalidLatitudeError", "InvalidLifetime", + "InvalidLifetimeError", "InvalidLongitude", "InvalidLongitudeError", "Lifetime", diff --git a/src/frequenz/client/common/types/_lifetime.py b/src/frequenz/client/common/types/_lifetime.py index a0c6b824..db35fad7 100644 --- a/src/frequenz/client/common/types/_lifetime.py +++ b/src/frequenz/client/common/types/_lifetime.py @@ -7,6 +7,8 @@ from datetime import datetime, timezone from typing import Any, Self +from .._exception import InvalidAttributeError + @dataclass(frozen=True, kw_only=True) class BaseLifetime: @@ -96,3 +98,42 @@ class InvalidLifetime(BaseLifetime): semantic accessor, such as `ElectricalComponent.get_operational_lifetime()`, to receive a clear [`InvalidLifetimeError`][..InvalidLifetimeError]. """ + + +class InvalidLifetimeError(InvalidAttributeError): + """Raised when a semantic accessor sees an invalid lifetime. + + The offending [`InvalidLifetime`][..InvalidLifetime] is available as the + [`lifetime`][.lifetime] attribute so callers can inspect the raw wire data. + + This is also a [`ValueError`][] for convenience. + """ + + def __init__( + self, + instance: object, + attr_name: str, + lifetime: InvalidLifetime, + message: str | None = None, + ) -> None: + """Initialize this error. + + Args: + instance: The instance that was being accessed when this error was raised. + attr_name: The name of the attribute that was being accessed. + lifetime: The invalid lifetime instance. + message: A custom error message. If `None`, a default message mentioning + the invalid lifetime is used. + """ + self.lifetime: InvalidLifetime = lifetime + """The invalid lifetime that caused this error.""" + + super().__init__( + instance, + attr_name, + ( + message + if message is not None + else f"invalid lifetime {lifetime!r} for attribute {attr_name!r} in {instance}" + ), + ) diff --git a/tests/types/test_lifetime.py b/tests/types/test_lifetime.py index b5e988ce..c6942eb0 100644 --- a/tests/types/test_lifetime.py +++ b/tests/types/test_lifetime.py @@ -9,7 +9,13 @@ import pytest -from frequenz.client.common.types import BaseLifetime, InvalidLifetime, Lifetime +from frequenz.client.common import InvalidAttributeError +from frequenz.client.common.types import ( + BaseLifetime, + InvalidLifetime, + InvalidLifetimeError, + Lifetime, +) class _Time(Enum): @@ -123,6 +129,47 @@ def test_invalid_lifetime_accepts_invalid_range( assert lifetime.end_time is present +def test_invalid_lifetime_error_default_message( + present: datetime, future: datetime +) -> None: + """`InvalidLifetimeError` builds a default message from the invalid lifetime.""" + invalid = InvalidLifetime(start_time=future, end_time=present) + error = InvalidLifetimeError("some-instance", "operational_lifetime", invalid) + + assert error.lifetime is invalid + assert ( + str(error) + == f"invalid lifetime {invalid!r} for attribute 'operational_lifetime' " + "in some-instance" + ) + + +def test_invalid_lifetime_error_custom_message( + present: datetime, future: datetime +) -> None: + """`InvalidLifetimeError` accepts a custom message.""" + invalid = InvalidLifetime(start_time=future, end_time=present) + error = InvalidLifetimeError( + "some-instance", + "operational_lifetime", + invalid, + message="bad lifetime from server", + ) + + assert error.lifetime is invalid + assert str(error) == "bad lifetime from server" + + +def test_invalid_lifetime_error_is_invalid_attribute_error() -> None: + """`InvalidLifetimeError` is an `InvalidAttributeError`.""" + assert issubclass(InvalidLifetimeError, InvalidAttributeError) + + +def test_invalid_lifetime_error_is_value_error() -> None: + """`InvalidLifetimeError` is a `ValueError`.""" + assert issubclass(InvalidLifetimeError, ValueError) + + @pytest.mark.parametrize( "case", [ From 6c43306529521124154b3d7b8ce1665d99b64891 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Mon, 13 Jul 2026 12:50:05 +0000 Subject: [PATCH 06/12] Update `lifetime_from_proto` to return `InvalidLifetime` Now the conversion function returns `Lifetime | InvalidLifetime` instead of raising for malformed timestamp ordering. Invalid wire data becomes an `InvalidLifetime` carrying both timestamps, so callers can inspect or report what the server sent. The existing converter retains its current raising behavior for now. Signed-off-by: Leandro Lucarella --- .../proto/v1alpha8/_electrical_component.py | 16 +++++++++++-- .../_electrical_component_connection.py | 17 +++++++++++-- .../common/types/proto/v1alpha8/_lifetime.py | 19 +++++++++------ tests/types/proto/v1alpha8/test_lifetime.py | 24 +++++++++++++------ 4 files changed, 58 insertions(+), 18 deletions(-) diff --git a/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component.py b/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component.py index d1c555e1..7e0797f2 100644 --- a/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component.py +++ b/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component.py @@ -16,7 +16,7 @@ from .....metrics import Bounds, Metric from .....metrics.proto.v1alpha8 import bounds_from_proto from .....proto import enum_from_proto -from .....types import Lifetime +from .....types import InvalidLifetime, Lifetime from .....types.proto.v1alpha8 import lifetime_from_proto from ...._ids import MicrogridId from ..._battery import ( @@ -1285,12 +1285,24 @@ def _get_operational_lifetime_from_proto( """ if message.HasField("operational_lifetime"): try: - return lifetime_from_proto(message.operational_lifetime) + lifetime = lifetime_from_proto(message.operational_lifetime) except ValueError as exc: major_issues.append( f"invalid operational lifetime ({exc}), considering it as missing " "(i.e. always operational)", ) + else: + match lifetime: + case Lifetime() as valid: + return valid + case InvalidLifetime(start_time=start, end_time=end): + major_issues.append( + f"invalid operational lifetime (Start ({start}) must be before " + f"or equal to end ({end})), considering it as missing " + "(i.e. always operational)", + ) + case unknown: + assert_never(unknown) else: minor_issues.append( "missing operational lifetime, considering it always operational", diff --git a/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component_connection.py b/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component_connection.py index 018fdcde..7ab04c79 100644 --- a/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component_connection.py +++ b/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component_connection.py @@ -4,12 +4,13 @@ """Loading of ElectricalComponentConnection objects from protobuf messages.""" import logging +from typing import assert_never from frequenz.api.common.v1alpha8.microgrid.electrical_components import ( electrical_components_pb2, ) -from .....types import Lifetime +from .....types import InvalidLifetime, Lifetime from .....types.proto.v1alpha8 import lifetime_from_proto from ... import ( ElectricalComponentConnection, @@ -119,12 +120,24 @@ def _get_operational_lifetime_from_proto( """ if message.HasField("operational_lifetime"): try: - return lifetime_from_proto(message.operational_lifetime) + lifetime = lifetime_from_proto(message.operational_lifetime) except ValueError as exc: major_issues.append( f"invalid operational lifetime ({exc}), considering it as missing " "(i.e. always operational)", ) + else: + match lifetime: + case Lifetime() as valid: + return valid + case InvalidLifetime(start_time=start, end_time=end): + major_issues.append( + f"invalid operational lifetime (Start ({start}) must be before " + f"or equal to end ({end})), considering it as missing " + "(i.e. always operational)", + ) + case unknown: + assert_never(unknown) else: minor_issues.append( "missing operational lifetime, considering it always operational", diff --git a/src/frequenz/client/common/types/proto/v1alpha8/_lifetime.py b/src/frequenz/client/common/types/proto/v1alpha8/_lifetime.py index 04f3b272..9c6451e0 100644 --- a/src/frequenz/client/common/types/proto/v1alpha8/_lifetime.py +++ b/src/frequenz/client/common/types/proto/v1alpha8/_lifetime.py @@ -6,19 +6,20 @@ from frequenz.api.common.v1alpha8.microgrid import lifetime_pb2 from ....proto import datetime_from_proto -from ..._lifetime import Lifetime +from ..._lifetime import InvalidLifetime, Lifetime -def lifetime_from_proto( - message: lifetime_pb2.Lifetime, -) -> Lifetime: - """Create a [`Lifetime`][....Lifetime] from a protobuf message. +def lifetime_from_proto(message: lifetime_pb2.Lifetime) -> Lifetime | InvalidLifetime: + """Create a lifetime from a protobuf message, preserving malformed data. Args: message: The protobuf message to convert. Returns: - The corresponding [`Lifetime`][....Lifetime] object. + A [`Lifetime`][....Lifetime] when the timestamps form a valid range, or + an [`InvalidLifetime`][....InvalidLifetime] preserving malformed + timestamp ordering. A present but empty protobuf message becomes an + unbounded `Lifetime()`. """ start = ( datetime_from_proto(message.start_timestamp) @@ -30,4 +31,8 @@ def lifetime_from_proto( if message.HasField("end_timestamp") else None ) - return Lifetime(start_time=start, end_time=end) + try: + return Lifetime(start_time=start, end_time=end) + except ValueError: + pass + return InvalidLifetime(start_time=start, end_time=end) diff --git a/tests/types/proto/v1alpha8/test_lifetime.py b/tests/types/proto/v1alpha8/test_lifetime.py index f30b8151..6779cbd3 100644 --- a/tests/types/proto/v1alpha8/test_lifetime.py +++ b/tests/types/proto/v1alpha8/test_lifetime.py @@ -11,6 +11,7 @@ from frequenz.api.common.v1alpha8.microgrid import lifetime_pb2 from google.protobuf import timestamp_pb2 +from frequenz.client.common.types import InvalidLifetime from frequenz.client.common.types.proto.v1alpha8 import lifetime_from_proto @@ -88,20 +89,29 @@ def test_from_proto( assert lifetime.end_time is None -def test_from_proto_rejects_start_after_end(now: datetime, future: datetime) -> None: - """Test conversion rejects protobuf messages with start after end.""" +@pytest.fixture +def invalid_lifetime_proto(now: datetime, future: datetime) -> lifetime_pb2.Lifetime: + """Provide a protobuf lifetime whose start timestamp is after its end.""" start_ts = timestamp_pb2.Timestamp() start_ts.FromDatetime(future) end_ts = timestamp_pb2.Timestamp() end_ts.FromDatetime(now) - proto = lifetime_pb2.Lifetime( + return lifetime_pb2.Lifetime( start_timestamp=start_ts, end_timestamp=end_ts, ) - with pytest.raises( - ValueError, match=r"Start \(.*\) must be before or equal to end \(.*\)" - ): - lifetime_from_proto(proto) + +def test_from_proto_preserves_start_after_end( + now: datetime, + future: datetime, + invalid_lifetime_proto: lifetime_pb2.Lifetime, +) -> None: + """The converter preserves malformed ordering as `InvalidLifetime`.""" + lifetime = lifetime_from_proto(invalid_lifetime_proto) + + assert isinstance(lifetime, InvalidLifetime) + assert lifetime.start_time == future + assert lifetime.end_time == now From 5c729bacc8707f013f44287d0b37dfe0fc22dc8d Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Mon, 13 Jul 2026 17:52:58 +0000 Subject: [PATCH 07/12] Add safe accessors for operational lifetimes Loosen `ElectricalComponent.operational_lifetime` and `BaseElectricalComponentConnection.operational_lifetime` to `Lifetime | InvalidLifetime`, preserving malformed timestamp ordering received from protobuf instead of replacing it with an unbounded `Lifetime`. Add `get_operational_lifetime() -> Lifetime` to both class hierarchies so callers that require a valid lifetime receive either the valid value or an `InvalidLifetimeError` carrying the malformed one. The `is_operational_at()` and `is_operational_now()` shortcuts now use the safe accessor and raise the same error for malformed data. Missing and present-but-empty protobuf lifetimes continue to become `Lifetime()`, as both represent an unbounded lifetime in the upstream schema. Signed-off-by: Leandro Lucarella --- .../_electrical_component.py | 51 +++++++++++-- .../_electrical_component_connection.py | 68 ++++++++++++++--- .../proto/v1alpha8/_electrical_component.py | 45 ++++------- .../_electrical_component_connection.py | 47 ++++-------- .../test_electrical_component_base.py | 42 ++++++++--- .../test_electrical_component_connection.py | 68 +++++++++++------ .../test_electrical_component_base.py | 74 +++++++++++++++++-- .../test_electrical_component_connection.py | 74 ++++++++++++++++++- 8 files changed, 354 insertions(+), 115 deletions(-) diff --git a/src/frequenz/client/common/microgrid/electrical_components/_electrical_component.py b/src/frequenz/client/common/microgrid/electrical_components/_electrical_component.py index 31dba2ce..b5b5aa0d 100644 --- a/src/frequenz/client/common/microgrid/electrical_components/_electrical_component.py +++ b/src/frequenz/client/common/microgrid/electrical_components/_electrical_component.py @@ -10,7 +10,7 @@ from ..._exception import UnrecognizedEnumValueError, UnspecifiedEnumValueError from ...metrics import Bounds, Metric -from ...types import Lifetime +from ...types import InvalidLifetime, InvalidLifetimeError, Lifetime from .. import MicrogridId from ._ids import ElectricalComponentId @@ -34,8 +34,18 @@ class ElectricalComponent: # pylint: disable=too-many-instance-attributes This includes both the manufacturer and the model name. """ - operational_lifetime: Lifetime = dataclasses.field(default_factory=Lifetime) - """The operational lifetime of this electrical component.""" + operational_lifetime: Lifetime | InvalidLifetime = dataclasses.field( + default_factory=Lifetime + ) + """The operational lifetime of this electrical component. + + An [`InvalidLifetime`][frequenz.client.common.types.InvalidLifetime] preserves + malformed wire data. + + Tip: + Prefer [`get_operational_lifetime()`][..get_operational_lifetime] when + a valid lifetime is required. + """ _provides_telemetry: bool | int """Whether this component provides telemetry data. @@ -183,7 +193,26 @@ def accepts_control(self) -> bool: case unknown: assert_never(unknown) - def is_operational_at(self, timestamp: datetime) -> bool: + def get_operational_lifetime(self) -> Lifetime: + """Return the operational lifetime as a valid `Lifetime`. + + Returns: + The valid operational lifetime. + + Raises: + InvalidLifetimeError: If malformed lifetime data was received. The + offending value is available on the exception's `lifetime` + attribute. + """ + match self.operational_lifetime: + case InvalidLifetime() as invalid: + raise InvalidLifetimeError(self, "operational_lifetime", invalid) + case Lifetime() as valid: + return valid + case unknown: + assert_never(unknown) + + def is_operational_at(self, timestamp: datetime) -> bool: # noqa: DOC502 """Check whether this electrical component is operational at a specific timestamp. Args: @@ -191,14 +220,24 @@ def is_operational_at(self, timestamp: datetime) -> bool: Returns: Whether this electrical component is operational at the given timestamp. + + Raises: + InvalidLifetimeError: If malformed lifetime data was received. The + offending value is available on the exception's `lifetime` + attribute. """ - return self.operational_lifetime.is_operational_at(timestamp) + return self.get_operational_lifetime().is_operational_at(timestamp) - def is_operational_now(self) -> bool: + def is_operational_now(self) -> bool: # noqa: DOC502 """Check whether this electrical component is currently operational. Returns: Whether this electrical component is operational at the current time. + + Raises: + InvalidLifetimeError: If malformed lifetime data was received. The + offending value is available on the exception's `lifetime` + attribute. """ return self.is_operational_at(datetime.now(timezone.utc)) diff --git a/src/frequenz/client/common/microgrid/electrical_components/_electrical_component_connection.py b/src/frequenz/client/common/microgrid/electrical_components/_electrical_component_connection.py index 0566d233..5f28f753 100644 --- a/src/frequenz/client/common/microgrid/electrical_components/_electrical_component_connection.py +++ b/src/frequenz/client/common/microgrid/electrical_components/_electrical_component_connection.py @@ -5,9 +5,9 @@ import dataclasses from datetime import datetime, timezone -from typing import Any, Self +from typing import Any, Self, assert_never -from ...types import Lifetime +from ...types import InvalidLifetime, InvalidLifetimeError, Lifetime from ._ids import ElectricalComponentId @@ -55,8 +55,18 @@ class BaseElectricalComponentConnection: This is the electrical component towards which the current flows. """ - operational_lifetime: Lifetime = dataclasses.field(default_factory=Lifetime) - """The operational lifetime of the connection.""" + operational_lifetime: Lifetime | InvalidLifetime = dataclasses.field( + default_factory=Lifetime + ) + """The operational lifetime of the connection. + + An [`InvalidLifetime`][frequenz.client.common.types.InvalidLifetime] preserves + malformed wire data. + + Tip: + Prefer [`get_operational_lifetime()`][..get_operational_lifetime] when + a valid lifetime is required. + """ # pylint: disable-next=unused-argument def __new__(cls, *args: Any, **kwargs: Any) -> Self: @@ -65,12 +75,52 @@ def __new__(cls, *args: Any, **kwargs: Any) -> Self: raise TypeError(f"Cannot instantiate {cls.__name__} directly") return super().__new__(cls) - def is_operational_at(self, timestamp: datetime) -> bool: - """Check whether this connection is operational at a specific timestamp.""" - return self.operational_lifetime.is_operational_at(timestamp) + def get_operational_lifetime(self) -> Lifetime: + """Return the operational lifetime as a valid `Lifetime`. + + Returns: + The valid operational lifetime. + + Raises: + InvalidLifetimeError: If malformed lifetime data was received. The + offending value is available on the exception's `lifetime` + attribute. + """ + match self.operational_lifetime: + case InvalidLifetime() as invalid: + raise InvalidLifetimeError(self, "operational_lifetime", invalid) + case Lifetime() as valid: + return valid + case unknown: + assert_never(unknown) + + def is_operational_at(self, timestamp: datetime) -> bool: # noqa: DOC502 + """Check whether this connection is operational at a specific timestamp. + + Args: + timestamp: The timestamp to check against the operational lifetime. - def is_operational_now(self) -> bool: - """Whether this connection is currently operational.""" + Returns: + Whether this connection is operational at the given timestamp. + + Raises: + InvalidLifetimeError: If malformed lifetime data was received. The + offending value is available on the exception's `lifetime` + attribute. + """ + return self.get_operational_lifetime().is_operational_at(timestamp) + + def is_operational_now(self) -> bool: # noqa: DOC502 + """Whether this connection is currently operational. + + Returns: + Whether this connection is operational at the current time. + + Raises: + InvalidLifetimeError: If malformed lifetime data was received. The + offending value is available on the exception's `lifetime` + attribute. + """ return self.is_operational_at(datetime.now(timezone.utc)) def __str__(self) -> str: diff --git a/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component.py b/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component.py index 7e0797f2..dfd47756 100644 --- a/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component.py +++ b/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component.py @@ -888,7 +888,7 @@ class _ElectricalComponentBaseData(NamedTuple): category: ElectricalComponentCategory | int """The category of the electrical component.""" - lifetime: Lifetime + lifetime: Lifetime | InvalidLifetime """The operational lifetime of the electrical component.""" metric_config_bounds: dict[Metric | int, Bounds] @@ -934,7 +934,7 @@ def _electrical_component_base_from_proto_with_issues( ) lifetime = _get_operational_lifetime_from_proto( - message, major_issues=major_issues, minor_issues=minor_issues + message, major_issues=major_issues ) metric_config_bounds = _metric_config_bounds_from_proto( @@ -1270,41 +1270,26 @@ def _get_operational_lifetime_from_proto( message: electrical_components_pb2.ElectricalComponent, *, major_issues: list[str], - minor_issues: list[str], -) -> Lifetime: +) -> Lifetime | InvalidLifetime: """Get the operational lifetime from a protobuf message. Args: message: The protobuf message to extract the operational lifetime from. major_issues: A list to collect major issues found during parsing. - minor_issues: A list to collect minor issues found during parsing. Returns: - The extracted operational lifetime, or an empty lifetime if the protobuf - field is missing or invalid. + The extracted operational lifetime, an invalid lifetime preserving + malformed timestamp ordering, or an unbounded lifetime if the field + is missing. """ if message.HasField("operational_lifetime"): - try: - lifetime = lifetime_from_proto(message.operational_lifetime) - except ValueError as exc: - major_issues.append( - f"invalid operational lifetime ({exc}), considering it as missing " - "(i.e. always operational)", - ) - else: - match lifetime: - case Lifetime() as valid: - return valid - case InvalidLifetime(start_time=start, end_time=end): - major_issues.append( - f"invalid operational lifetime (Start ({start}) must be before " - f"or equal to end ({end})), considering it as missing " - "(i.e. always operational)", - ) - case unknown: - assert_never(unknown) - else: - minor_issues.append( - "missing operational lifetime, considering it always operational", - ) + lifetime = lifetime_from_proto(message.operational_lifetime) + match lifetime: + case InvalidLifetime(): + major_issues.append("invalid operational lifetime") + case Lifetime(): + pass + case unknown: + assert_never(unknown) + return lifetime return Lifetime() diff --git a/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component_connection.py b/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component_connection.py index 7ab04c79..aeca2504 100644 --- a/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component_connection.py +++ b/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component_connection.py @@ -75,13 +75,13 @@ def electrical_component_connection_from_proto_with_issues( Returns: One of the concrete connection types. """ + del minor_issues + source_component_id = ElectricalComponentId(message.source_electrical_component_id) destination_component_id = ElectricalComponentId( message.destination_electrical_component_id ) - lifetime = _get_operational_lifetime_from_proto( - message, major_issues=major_issues, minor_issues=minor_issues - ) + lifetime = _get_operational_lifetime_from_proto(message, major_issues=major_issues) if source_component_id == destination_component_id: major_issues.append( @@ -105,41 +105,26 @@ def _get_operational_lifetime_from_proto( message: electrical_components_pb2.ElectricalComponentConnection, *, major_issues: list[str], - minor_issues: list[str], -) -> Lifetime: +) -> Lifetime | InvalidLifetime: """Get the operational lifetime from a protobuf message. Args: message: The protobuf message to extract the operational lifetime from. major_issues: A list to collect major issues found during parsing. - minor_issues: A list to collect minor issues found during parsing. Returns: - The extracted operational lifetime, or an empty lifetime if the protobuf - field is missing or invalid. + The extracted operational lifetime, an invalid lifetime preserving + malformed timestamp ordering, or an unbounded lifetime if the field + is missing. """ if message.HasField("operational_lifetime"): - try: - lifetime = lifetime_from_proto(message.operational_lifetime) - except ValueError as exc: - major_issues.append( - f"invalid operational lifetime ({exc}), considering it as missing " - "(i.e. always operational)", - ) - else: - match lifetime: - case Lifetime() as valid: - return valid - case InvalidLifetime(start_time=start, end_time=end): - major_issues.append( - f"invalid operational lifetime (Start ({start}) must be before " - f"or equal to end ({end})), considering it as missing " - "(i.e. always operational)", - ) - case unknown: - assert_never(unknown) - else: - minor_issues.append( - "missing operational lifetime, considering it always operational", - ) + lifetime = lifetime_from_proto(message.operational_lifetime) + match lifetime: + case InvalidLifetime(): + major_issues.append("invalid operational lifetime") + case Lifetime(): + pass + case unknown: + assert_never(unknown) + return lifetime return Lifetime() diff --git a/tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_base.py b/tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_base.py index dc1e7079..3953c98b 100644 --- a/tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_base.py +++ b/tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_base.py @@ -3,6 +3,8 @@ """Tests for protobuf conversion of the base/common part of electrical components.""" +from datetime import timezone + import pytest from frequenz.api.common.v1alpha8.metrics import bounds_pb2, metrics_pb2 from frequenz.api.common.v1alpha8.microgrid.electrical_components import ( @@ -20,7 +22,7 @@ _metric_config_bounds_from_proto, _operational_mode_to_bools, ) -from frequenz.client.common.types import Lifetime +from frequenz.client.common.types import InvalidLifetime, Lifetime from .conftest import base_data_as_proto @@ -111,11 +113,29 @@ def test_missing_category_specific_info( ) assert sorted(major_issues) == sorted(["category is unspecified"]) - assert sorted(minor_issues) == sorted( - [ - "missing operational lifetime, considering it always operational", - ] + assert not minor_issues + assert parsed == base_data + + +def test_empty_lifetime_is_unbounded( + default_component_base_data: _ElectricalComponentBaseData, +) -> None: + """A present but empty protobuf lifetime becomes an unbounded `Lifetime`.""" + major_issues: list[str] = [] + minor_issues: list[str] = [] + base_data = default_component_base_data._replace( + category=ElectricalComponentCategory.CHP, + lifetime=Lifetime(), ) + proto = base_data_as_proto(base_data) + + assert proto.HasField("operational_lifetime") + parsed = _electrical_component_base_from_proto_with_issues( + proto, major_issues=major_issues, minor_issues=minor_issues + ) + + assert not major_issues + assert not minor_issues assert parsed == base_data @@ -153,7 +173,11 @@ def test_invalid_lifetime( major_issues: list[str] = [] minor_issues: list[str] = [] base_data = default_component_base_data._replace( - category=ElectricalComponentCategory.CHP, lifetime=Lifetime() + category=ElectricalComponentCategory.CHP, + lifetime=InvalidLifetime( + start_time=Timestamp(seconds=1696204800).ToDatetime(tzinfo=timezone.utc), + end_time=Timestamp(seconds=1696118400).ToDatetime(tzinfo=timezone.utc), + ), ) proto = base_data_as_proto(base_data) proto.operational_lifetime.start_timestamp.CopyFrom( @@ -167,11 +191,7 @@ def test_invalid_lifetime( proto, major_issues=major_issues, minor_issues=minor_issues ) - assert major_issues == [ - "invalid operational lifetime (Start (2023-10-02 00:00:00+00:00) must be " - "before or equal to end (2023-10-01 00:00:00+00:00)), considering it as " - "missing (i.e. always operational)" - ] + assert major_issues == ["invalid operational lifetime"] assert not minor_issues assert parsed == base_data diff --git a/tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_connection.py b/tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_connection.py index a52aa077..e08309ce 100644 --- a/tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_connection.py +++ b/tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_connection.py @@ -24,6 +24,7 @@ electrical_component_connection_from_proto, electrical_component_connection_from_proto_with_issues, ) +from frequenz.client.common.types import InvalidLifetime, Lifetime @pytest.mark.parametrize( @@ -44,7 +45,7 @@ "destination_electrical_component_id": 2, "has_lifetime": False, }, - ["missing operational lifetime, considering it always operational"], + [], id="no_lifetime", ), ], @@ -83,6 +84,31 @@ def test_success(proto_data: dict[str, Any], expected_minor_issues: list[str]) - assert connection.destination_id == ElectricalComponentId( proto_data["destination_electrical_component_id"] ) + if proto_data["has_lifetime"]: + assert isinstance(connection.operational_lifetime, Lifetime) + assert connection.operational_lifetime.start_time is not None + else: + assert connection.operational_lifetime == Lifetime() + + +def test_empty_lifetime_is_unbounded() -> None: + """A present but empty protobuf lifetime becomes an unbounded `Lifetime`.""" + proto = electrical_components_pb2.ElectricalComponentConnection( + source_electrical_component_id=1, + destination_electrical_component_id=2, + operational_lifetime=lifetime_pb2.Lifetime(), + ) + major_issues: list[str] = [] + minor_issues: list[str] = [] + + assert proto.HasField("operational_lifetime") + connection = electrical_component_connection_from_proto_with_issues( + proto, major_issues=major_issues, minor_issues=minor_issues + ) + + assert connection.operational_lifetime == Lifetime() + assert not major_issues + assert not minor_issues def test_self_referencing_same_ids() -> None: @@ -105,29 +131,24 @@ def test_self_referencing_same_ids() -> None: assert major_issues == [ "self-referencing connection: source and destination are the same (CID1)" ] - assert minor_issues == [ - "missing operational lifetime, considering it always operational" - ] + assert not minor_issues -@patch( - "frequenz.client.common.microgrid.electrical_components.proto.v1alpha8." - "_electrical_component_connection.lifetime_from_proto", - autospec=True, -) -def test_invalid_lifetime(mock_lifetime_from_proto: Mock) -> None: +def test_invalid_lifetime() -> None: """Test proto conversion with invalid lifetime data.""" - mock_lifetime_from_proto.side_effect = ValueError("Invalid lifetime") - proto = electrical_components_pb2.ElectricalComponentConnection( source_electrical_component_id=1, destination_electrical_component_id=2 ) - now = datetime.now(timezone.utc) start_time = timestamp_pb2.Timestamp() - start_time.FromDatetime(now) - lifetime = lifetime_pb2.Lifetime() - lifetime.start_timestamp.CopyFrom(start_time) - proto.operational_lifetime.CopyFrom(lifetime) + start_time.FromDatetime(datetime(2025, 2, 1, tzinfo=timezone.utc)) + end_time = timestamp_pb2.Timestamp() + end_time.FromDatetime(datetime(2025, 1, 1, tzinfo=timezone.utc)) + proto.operational_lifetime.CopyFrom( + lifetime_pb2.Lifetime( + start_timestamp=start_time, + end_timestamp=end_time, + ) + ) major_issues: list[str] = [] minor_issues: list[str] = [] @@ -138,12 +159,15 @@ def test_invalid_lifetime(mock_lifetime_from_proto: Mock) -> None: assert connection is not None assert connection.source_id == ElectricalComponentId(1) assert connection.destination_id == ElectricalComponentId(2) - assert major_issues == [ - "invalid operational lifetime (Invalid lifetime), considering it as missing " - "(i.e. always operational)" - ] + assert isinstance(connection.operational_lifetime, InvalidLifetime) + assert connection.operational_lifetime.start_time == datetime( + 2025, 2, 1, tzinfo=timezone.utc + ) + assert connection.operational_lifetime.end_time == datetime( + 2025, 1, 1, tzinfo=timezone.utc + ) + assert major_issues == ["invalid operational lifetime"] assert not minor_issues - mock_lifetime_from_proto.assert_called_once_with(proto.operational_lifetime) @patch( diff --git a/tests/microgrid/electrical_components/test_electrical_component_base.py b/tests/microgrid/electrical_components/test_electrical_component_base.py index 241289ff..cb5dbe0e 100644 --- a/tests/microgrid/electrical_components/test_electrical_component_base.py +++ b/tests/microgrid/electrical_components/test_electrical_component_base.py @@ -8,23 +8,40 @@ import pytest -from frequenz.client.common import ( - UnrecognizedEnumValueError, - UnspecifiedEnumValueError, -) +from frequenz.client.common import UnrecognizedEnumValueError, UnspecifiedEnumValueError from frequenz.client.common.metrics import Bounds, Metric from frequenz.client.common.microgrid import MicrogridId from frequenz.client.common.microgrid.electrical_components import ( ElectricalComponent, ElectricalComponentId, ) -from frequenz.client.common.types import Lifetime +from frequenz.client.common.types import ( + InvalidLifetime, + InvalidLifetimeError, + Lifetime, +) class _TestElectricalComponent(ElectricalComponent): """A simple electrical component implementation for testing.""" +def _make_component( + operational_lifetime: Lifetime | InvalidLifetime, +) -> _TestElectricalComponent: + """Build a test component with the given operational lifetime.""" + return _TestElectricalComponent( + id=ElectricalComponentId(1), + microgrid_id=MicrogridId(2), + name="", + model="Test Model", + operational_lifetime=operational_lifetime, + _provides_telemetry=True, + _accepts_control=True, + _allow_construction=True, + ) + + def test_base_creation_fails() -> None: """Test that ElectricalComponent base class cannot be instantiated directly.""" with pytest.raises( @@ -150,6 +167,53 @@ def test_accessors_raise_when_unrecognized() -> None: assert exc_info.value.value == 999 +def test_get_operational_lifetime_returns_valid() -> None: + """`get_operational_lifetime()` returns a valid lifetime unchanged.""" + lifetime = Lifetime() + component = _make_component(lifetime) + + assert component.get_operational_lifetime() is lifetime + + +def test_get_operational_lifetime_raises_invalid() -> None: + """`get_operational_lifetime()` raises with the malformed lifetime attached.""" + invalid = InvalidLifetime( + start_time=datetime(2025, 2, 1, tzinfo=timezone.utc), + end_time=datetime(2025, 1, 1, tzinfo=timezone.utc), + ) + component = _make_component(invalid) + + with pytest.raises(InvalidLifetimeError) as exc_info: + component.get_operational_lifetime() + assert exc_info.value.lifetime is invalid + + +def test_is_operational_at_raises_for_invalid_lifetime() -> None: + """`is_operational_at()` raises when the lifetime is invalid.""" + component = _make_component( + InvalidLifetime( + start_time=datetime(2025, 2, 1, tzinfo=timezone.utc), + end_time=datetime(2025, 1, 1, tzinfo=timezone.utc), + ) + ) + + with pytest.raises(InvalidLifetimeError): + component.is_operational_at(datetime(2025, 1, 1, tzinfo=timezone.utc)) + + +def test_is_operational_now_raises_for_invalid_lifetime() -> None: + """`is_operational_now()` raises when the lifetime is invalid.""" + component = _make_component( + InvalidLifetime( + start_time=datetime(2025, 2, 1, tzinfo=timezone.utc), + end_time=datetime(2025, 1, 1, tzinfo=timezone.utc), + ) + ) + + with pytest.raises(InvalidLifetimeError): + component.is_operational_now() + + @pytest.mark.parametrize( "name,expected_str", [ diff --git a/tests/microgrid/electrical_components/test_electrical_component_connection.py b/tests/microgrid/electrical_components/test_electrical_component_connection.py index 77dbbc64..eefeccc0 100644 --- a/tests/microgrid/electrical_components/test_electrical_component_connection.py +++ b/tests/microgrid/electrical_components/test_electrical_component_connection.py @@ -13,7 +13,22 @@ ElectricalComponentConnection, ElectricalComponentId, ) -from frequenz.client.common.types import Lifetime +from frequenz.client.common.types import ( + InvalidLifetime, + InvalidLifetimeError, + Lifetime, +) + + +def _make_connection( + operational_lifetime: Lifetime | InvalidLifetime, +) -> ElectricalComponentConnection: + """Build a connection with the given operational lifetime.""" + return ElectricalComponentConnection( + source_id=ElectricalComponentId(1), + destination_id=ElectricalComponentId(2), + operational_lifetime=operational_lifetime, + ) def test_abstract_base_cannot_be_instantiated() -> None: @@ -42,6 +57,63 @@ def test_creation() -> None: assert connection.operational_lifetime == lifetime +def test_creation_default_lifetime_is_unbounded() -> None: + """A connection with no lifetime stores an unbounded lifetime.""" + connection = ElectricalComponentConnection( + source_id=ElectricalComponentId(1), + destination_id=ElectricalComponentId(2), + ) + + assert connection.operational_lifetime == Lifetime() + + +def test_get_operational_lifetime_returns_valid() -> None: + """`get_operational_lifetime()` returns a valid lifetime unchanged.""" + lifetime = Lifetime() + connection = _make_connection(lifetime) + + assert connection.get_operational_lifetime() is lifetime + + +def test_get_operational_lifetime_raises_invalid() -> None: + """`get_operational_lifetime()` raises with the malformed lifetime attached.""" + invalid = InvalidLifetime( + start_time=datetime(2025, 2, 1, tzinfo=timezone.utc), + end_time=datetime(2025, 1, 1, tzinfo=timezone.utc), + ) + connection = _make_connection(invalid) + + with pytest.raises(InvalidLifetimeError) as exc_info: + connection.get_operational_lifetime() + assert exc_info.value.lifetime is invalid + + +def test_is_operational_at_raises_for_invalid_lifetime() -> None: + """`is_operational_at()` raises when the lifetime is invalid.""" + connection = _make_connection( + InvalidLifetime( + start_time=datetime(2025, 2, 1, tzinfo=timezone.utc), + end_time=datetime(2025, 1, 1, tzinfo=timezone.utc), + ) + ) + + with pytest.raises(InvalidLifetimeError): + connection.is_operational_at(datetime(2025, 1, 1, tzinfo=timezone.utc)) + + +def test_is_operational_now_raises_for_invalid_lifetime() -> None: + """`is_operational_now()` raises when the lifetime is invalid.""" + connection = _make_connection( + InvalidLifetime( + start_time=datetime(2025, 2, 1, tzinfo=timezone.utc), + end_time=datetime(2025, 1, 1, tzinfo=timezone.utc), + ) + ) + + with pytest.raises(InvalidLifetimeError): + connection.is_operational_now() + + def test_validation() -> None: """Test validation of source and destination electrical components.""" with pytest.raises( From dc6a5414b231989564a07a8967598d81cb6815a7 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Tue, 14 Jul 2026 09:43:25 +0200 Subject: [PATCH 08/12] Stop reporting lifetime issues via strings Now that invalid lifetimes are encoded into the type system, we don't need to report issues with them via the major/minor issues side-channel. Signed-off-by: Leandro Lucarella --- .../proto/v1alpha8/_electrical_component.py | 17 ++--------------- .../_electrical_component_connection.py | 16 ++-------------- .../v1alpha8/test_electrical_component_base.py | 2 +- .../test_electrical_component_connection.py | 2 +- 4 files changed, 6 insertions(+), 31 deletions(-) diff --git a/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component.py b/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component.py index dfd47756..04460d0b 100644 --- a/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component.py +++ b/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component.py @@ -933,9 +933,7 @@ def _electrical_component_base_from_proto_with_issues( message.operational_mode ) - lifetime = _get_operational_lifetime_from_proto( - message, major_issues=major_issues - ) + lifetime = _get_operational_lifetime_from_proto(message) metric_config_bounds = _metric_config_bounds_from_proto( message.metric_config_bounds, @@ -1268,14 +1266,11 @@ def _metric_config_bounds_from_proto( def _get_operational_lifetime_from_proto( message: electrical_components_pb2.ElectricalComponent, - *, - major_issues: list[str], ) -> Lifetime | InvalidLifetime: """Get the operational lifetime from a protobuf message. Args: message: The protobuf message to extract the operational lifetime from. - major_issues: A list to collect major issues found during parsing. Returns: The extracted operational lifetime, an invalid lifetime preserving @@ -1283,13 +1278,5 @@ def _get_operational_lifetime_from_proto( is missing. """ if message.HasField("operational_lifetime"): - lifetime = lifetime_from_proto(message.operational_lifetime) - match lifetime: - case InvalidLifetime(): - major_issues.append("invalid operational lifetime") - case Lifetime(): - pass - case unknown: - assert_never(unknown) - return lifetime + return lifetime_from_proto(message.operational_lifetime) return Lifetime() diff --git a/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component_connection.py b/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component_connection.py index aeca2504..e4ce94f8 100644 --- a/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component_connection.py +++ b/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component_connection.py @@ -4,7 +4,6 @@ """Loading of ElectricalComponentConnection objects from protobuf messages.""" import logging -from typing import assert_never from frequenz.api.common.v1alpha8.microgrid.electrical_components import ( electrical_components_pb2, @@ -81,7 +80,7 @@ def electrical_component_connection_from_proto_with_issues( destination_component_id = ElectricalComponentId( message.destination_electrical_component_id ) - lifetime = _get_operational_lifetime_from_proto(message, major_issues=major_issues) + lifetime = _get_operational_lifetime_from_proto(message) if source_component_id == destination_component_id: major_issues.append( @@ -103,14 +102,11 @@ def electrical_component_connection_from_proto_with_issues( def _get_operational_lifetime_from_proto( message: electrical_components_pb2.ElectricalComponentConnection, - *, - major_issues: list[str], ) -> Lifetime | InvalidLifetime: """Get the operational lifetime from a protobuf message. Args: message: The protobuf message to extract the operational lifetime from. - major_issues: A list to collect major issues found during parsing. Returns: The extracted operational lifetime, an invalid lifetime preserving @@ -118,13 +114,5 @@ def _get_operational_lifetime_from_proto( is missing. """ if message.HasField("operational_lifetime"): - lifetime = lifetime_from_proto(message.operational_lifetime) - match lifetime: - case InvalidLifetime(): - major_issues.append("invalid operational lifetime") - case Lifetime(): - pass - case unknown: - assert_never(unknown) - return lifetime + return lifetime_from_proto(message.operational_lifetime) return Lifetime() diff --git a/tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_base.py b/tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_base.py index 3953c98b..2f8fa301 100644 --- a/tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_base.py +++ b/tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_base.py @@ -191,7 +191,7 @@ def test_invalid_lifetime( proto, major_issues=major_issues, minor_issues=minor_issues ) - assert major_issues == ["invalid operational lifetime"] + assert not major_issues assert not minor_issues assert parsed == base_data diff --git a/tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_connection.py b/tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_connection.py index e08309ce..d191facb 100644 --- a/tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_connection.py +++ b/tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_connection.py @@ -166,7 +166,7 @@ def test_invalid_lifetime() -> None: assert connection.operational_lifetime.end_time == datetime( 2025, 1, 1, tzinfo=timezone.utc ) - assert major_issues == ["invalid operational lifetime"] + assert not major_issues assert not minor_issues From b888962da698dfd29f0703191464dc39771f22c6 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Mon, 13 Jul 2026 17:57:10 +0000 Subject: [PATCH 09/12] Split `Lifetime` tests into per-type files `src/.../types/_lifetime.py` now exposes four public symbols, so the test file grew too large. Split it following the repository convention: tests/types/test_lifetime.py -> tests/types/_lifetime/test_.py Test bodies and assertions are unchanged. Shared time fixtures move to the subdirectory's `conftest.py`, and test names drop redundant type prefixes. Signed-off-by: Leandro Lucarella --- tests/types/_lifetime/__init__.py | 4 + tests/types/_lifetime/conftest.py | 26 +++++ tests/types/_lifetime/test_base_lifetime.py | 14 +++ .../types/_lifetime/test_invalid_lifetime.py | 21 +++++ .../_lifetime/test_invalid_lifetime_error.py | 46 +++++++++ tests/types/{ => _lifetime}/test_lifetime.py | 94 +------------------ 6 files changed, 115 insertions(+), 90 deletions(-) create mode 100644 tests/types/_lifetime/__init__.py create mode 100644 tests/types/_lifetime/conftest.py create mode 100644 tests/types/_lifetime/test_base_lifetime.py create mode 100644 tests/types/_lifetime/test_invalid_lifetime.py create mode 100644 tests/types/_lifetime/test_invalid_lifetime_error.py rename tests/types/{ => _lifetime}/test_lifetime.py (75%) diff --git a/tests/types/_lifetime/__init__.py b/tests/types/_lifetime/__init__.py new file mode 100644 index 00000000..5db2bdc4 --- /dev/null +++ b/tests/types/_lifetime/__init__.py @@ -0,0 +1,4 @@ +# License: MIT +# Copyright © 2026 Frequenz Energy-as-a-Service GmbH + +"""Tests for lifetime types.""" diff --git a/tests/types/_lifetime/conftest.py b/tests/types/_lifetime/conftest.py new file mode 100644 index 00000000..3acb3b77 --- /dev/null +++ b/tests/types/_lifetime/conftest.py @@ -0,0 +1,26 @@ +# License: MIT +# Copyright © 2026 Frequenz Energy-as-a-Service GmbH + +"""Fixtures for lifetime tests.""" + +from datetime import datetime, timezone + +import pytest + + +@pytest.fixture +def present() -> datetime: + """Provide the current UTC time.""" + return datetime.now(timezone.utc) + + +@pytest.fixture +def past(present: datetime) -> datetime: + """Provide a time in the past.""" + return present.replace(year=present.year - 1) + + +@pytest.fixture +def future(present: datetime) -> datetime: + """Provide a time in the future.""" + return present.replace(year=present.year + 1) diff --git a/tests/types/_lifetime/test_base_lifetime.py b/tests/types/_lifetime/test_base_lifetime.py new file mode 100644 index 00000000..f81627db --- /dev/null +++ b/tests/types/_lifetime/test_base_lifetime.py @@ -0,0 +1,14 @@ +# License: MIT +# Copyright © 2026 Frequenz Energy-as-a-Service GmbH + +"""Tests for `BaseLifetime`.""" + +import pytest + +from frequenz.client.common.types import BaseLifetime + + +def test_cannot_be_instantiated_directly() -> None: + """`BaseLifetime` refuses direct instantiation.""" + with pytest.raises(TypeError, match="Cannot instantiate BaseLifetime directly"): + BaseLifetime() diff --git a/tests/types/_lifetime/test_invalid_lifetime.py b/tests/types/_lifetime/test_invalid_lifetime.py new file mode 100644 index 00000000..74e385c5 --- /dev/null +++ b/tests/types/_lifetime/test_invalid_lifetime.py @@ -0,0 +1,21 @@ +# License: MIT +# Copyright © 2026 Frequenz Energy-as-a-Service GmbH + +"""Tests for `InvalidLifetime`.""" + +from datetime import datetime + +from frequenz.client.common.types import BaseLifetime, InvalidLifetime + + +def test_is_base_lifetime_subclass() -> None: + """`InvalidLifetime` is a subclass of `BaseLifetime`.""" + assert issubclass(InvalidLifetime, BaseLifetime) + + +def test_accepts_invalid_range(present: datetime, future: datetime) -> None: + """`InvalidLifetime` preserves an end time before its start time.""" + lifetime = InvalidLifetime(start_time=future, end_time=present) + + assert lifetime.start_time is future + assert lifetime.end_time is present diff --git a/tests/types/_lifetime/test_invalid_lifetime_error.py b/tests/types/_lifetime/test_invalid_lifetime_error.py new file mode 100644 index 00000000..29de2b52 --- /dev/null +++ b/tests/types/_lifetime/test_invalid_lifetime_error.py @@ -0,0 +1,46 @@ +# License: MIT +# Copyright © 2026 Frequenz Energy-as-a-Service GmbH + +"""Tests for `InvalidLifetimeError`.""" + +from datetime import datetime + +from frequenz.client.common import InvalidAttributeError +from frequenz.client.common.types import InvalidLifetime, InvalidLifetimeError + + +def test_default_message(present: datetime, future: datetime) -> None: + """`InvalidLifetimeError` builds a default message from the invalid lifetime.""" + invalid = InvalidLifetime(start_time=future, end_time=present) + error = InvalidLifetimeError("some-instance", "operational_lifetime", invalid) + + assert error.lifetime is invalid + assert ( + str(error) + == f"invalid lifetime {invalid!r} for attribute 'operational_lifetime' " + "in some-instance" + ) + + +def test_custom_message(present: datetime, future: datetime) -> None: + """`InvalidLifetimeError` accepts a custom message.""" + invalid = InvalidLifetime(start_time=future, end_time=present) + error = InvalidLifetimeError( + "some-instance", + "operational_lifetime", + invalid, + message="bad lifetime from server", + ) + + assert error.lifetime is invalid + assert str(error) == "bad lifetime from server" + + +def test_is_invalid_attribute_error() -> None: + """`InvalidLifetimeError` is an `InvalidAttributeError`.""" + assert issubclass(InvalidLifetimeError, InvalidAttributeError) + + +def test_is_value_error() -> None: + """`InvalidLifetimeError` is a `ValueError`.""" + assert issubclass(InvalidLifetimeError, ValueError) diff --git a/tests/types/test_lifetime.py b/tests/types/_lifetime/test_lifetime.py similarity index 75% rename from tests/types/test_lifetime.py rename to tests/types/_lifetime/test_lifetime.py index c6942eb0..4a70d1ac 100644 --- a/tests/types/test_lifetime.py +++ b/tests/types/_lifetime/test_lifetime.py @@ -1,21 +1,15 @@ # License: MIT # Copyright © 2025 Frequenz Energy-as-a-Service GmbH -"""Tests for the Lifetime class.""" +"""Tests for `Lifetime`.""" from dataclasses import dataclass -from datetime import datetime, timezone +from datetime import datetime from enum import Enum, auto import pytest -from frequenz.client.common import InvalidAttributeError -from frequenz.client.common.types import ( - BaseLifetime, - InvalidLifetime, - InvalidLifetimeError, - Lifetime, -) +from frequenz.client.common.types import BaseLifetime, Lifetime class _Time(Enum): @@ -85,91 +79,11 @@ class _FixedLifetimeTestCase: """The expected operational state.""" -@pytest.fixture -def present() -> datetime: - """Fixture to provide current UTC time.""" - return datetime.now(timezone.utc) - - -@pytest.fixture -def past(present: datetime) -> datetime: - """Fixture to provide a past time.""" - return present.replace(year=present.year - 1) - - -@pytest.fixture -def future(present: datetime) -> datetime: - """Fixture to provide a future time.""" - return present.replace(year=present.year + 1) - - -def test_base_lifetime_cannot_be_instantiated_directly() -> None: - """`BaseLifetime` refuses direct instantiation.""" - with pytest.raises(TypeError, match="Cannot instantiate BaseLifetime directly"): - BaseLifetime() - - -def test_lifetime_is_base_lifetime_subclass() -> None: +def test_is_base_lifetime_subclass() -> None: """`Lifetime` is a subclass of `BaseLifetime`.""" assert issubclass(Lifetime, BaseLifetime) -def test_invalid_lifetime_is_base_lifetime_subclass() -> None: - """`InvalidLifetime` is a subclass of `BaseLifetime`.""" - assert issubclass(InvalidLifetime, BaseLifetime) - - -def test_invalid_lifetime_accepts_invalid_range( - present: datetime, future: datetime -) -> None: - """`InvalidLifetime` preserves an end time before its start time.""" - lifetime = InvalidLifetime(start_time=future, end_time=present) - - assert lifetime.start_time is future - assert lifetime.end_time is present - - -def test_invalid_lifetime_error_default_message( - present: datetime, future: datetime -) -> None: - """`InvalidLifetimeError` builds a default message from the invalid lifetime.""" - invalid = InvalidLifetime(start_time=future, end_time=present) - error = InvalidLifetimeError("some-instance", "operational_lifetime", invalid) - - assert error.lifetime is invalid - assert ( - str(error) - == f"invalid lifetime {invalid!r} for attribute 'operational_lifetime' " - "in some-instance" - ) - - -def test_invalid_lifetime_error_custom_message( - present: datetime, future: datetime -) -> None: - """`InvalidLifetimeError` accepts a custom message.""" - invalid = InvalidLifetime(start_time=future, end_time=present) - error = InvalidLifetimeError( - "some-instance", - "operational_lifetime", - invalid, - message="bad lifetime from server", - ) - - assert error.lifetime is invalid - assert str(error) == "bad lifetime from server" - - -def test_invalid_lifetime_error_is_invalid_attribute_error() -> None: - """`InvalidLifetimeError` is an `InvalidAttributeError`.""" - assert issubclass(InvalidLifetimeError, InvalidAttributeError) - - -def test_invalid_lifetime_error_is_value_error() -> None: - """`InvalidLifetimeError` is a `ValueError`.""" - assert issubclass(InvalidLifetimeError, ValueError) - - @pytest.mark.parametrize( "case", [ From a985835041541a03a43671d7253044e17dc07b7b Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Tue, 14 Jul 2026 11:05:27 +0200 Subject: [PATCH 10/12] Move `Lifetime` from `types` to `microgrid` This is to match where `Lifetime` is defined in the protobuf messages. Signed-off-by: Leandro Lucarella --- src/frequenz/client/common/microgrid/__init__.py | 10 ++++++++++ .../client/common/{types => microgrid}/_lifetime.py | 0 .../electrical_components/_electrical_component.py | 5 ++--- .../_electrical_component_connection.py | 5 ++--- .../proto/v1alpha8/_electrical_component.py | 4 ++-- .../v1alpha8/_electrical_component_connection.py | 4 ++-- .../common/microgrid/proto/v1alpha8/__init__.py | 2 ++ .../{types => microgrid}/proto/v1alpha8/_lifetime.py | 0 src/frequenz/client/common/types/__init__.py | 10 ---------- .../client/common/types/proto/v1alpha8/__init__.py | 2 -- tests/{types => microgrid}/_lifetime/__init__.py | 0 tests/{types => microgrid}/_lifetime/conftest.py | 0 .../_lifetime/test_base_lifetime.py | 2 +- .../_lifetime/test_invalid_lifetime.py | 2 +- .../_lifetime/test_invalid_lifetime_error.py | 2 +- .../{types => microgrid}/_lifetime/test_lifetime.py | 2 +- .../electrical_components/proto/v1alpha8/conftest.py | 3 +-- .../proto/v1alpha8/test_electrical_component_base.py | 2 +- .../v1alpha8/test_electrical_component_connection.py | 2 +- .../test_electrical_component_base.py | 12 ++++++------ .../test_electrical_component_connection.py | 10 +++++----- .../test_problematic_connection.py | 2 +- .../proto/v1alpha8/test_lifetime.py | 4 ++-- 23 files changed, 41 insertions(+), 44 deletions(-) rename src/frequenz/client/common/{types => microgrid}/_lifetime.py (100%) rename src/frequenz/client/common/{types => microgrid}/proto/v1alpha8/_lifetime.py (100%) rename tests/{types => microgrid}/_lifetime/__init__.py (100%) rename tests/{types => microgrid}/_lifetime/conftest.py (100%) rename tests/{types => microgrid}/_lifetime/test_base_lifetime.py (85%) rename tests/{types => microgrid}/_lifetime/test_invalid_lifetime.py (88%) rename tests/{types => microgrid}/_lifetime/test_invalid_lifetime_error.py (94%) rename tests/{types => microgrid}/_lifetime/test_lifetime.py (99%) rename tests/{types => microgrid}/proto/v1alpha8/test_lifetime.py (95%) diff --git a/src/frequenz/client/common/microgrid/__init__.py b/src/frequenz/client/common/microgrid/__init__.py index 50f3b60f..ee982607 100644 --- a/src/frequenz/client/common/microgrid/__init__.py +++ b/src/frequenz/client/common/microgrid/__init__.py @@ -4,10 +4,20 @@ """Frequenz microgrid definition.""" from ._ids import EnterpriseId, MicrogridId +from ._lifetime import ( + BaseLifetime, + InvalidLifetime, + InvalidLifetimeError, + Lifetime, +) from ._microgrid import Microgrid __all__ = [ + "BaseLifetime", "EnterpriseId", + "InvalidLifetime", + "InvalidLifetimeError", + "Lifetime", "Microgrid", "MicrogridId", ] diff --git a/src/frequenz/client/common/types/_lifetime.py b/src/frequenz/client/common/microgrid/_lifetime.py similarity index 100% rename from src/frequenz/client/common/types/_lifetime.py rename to src/frequenz/client/common/microgrid/_lifetime.py diff --git a/src/frequenz/client/common/microgrid/electrical_components/_electrical_component.py b/src/frequenz/client/common/microgrid/electrical_components/_electrical_component.py index b5b5aa0d..ee74b0a0 100644 --- a/src/frequenz/client/common/microgrid/electrical_components/_electrical_component.py +++ b/src/frequenz/client/common/microgrid/electrical_components/_electrical_component.py @@ -10,8 +10,8 @@ from ..._exception import UnrecognizedEnumValueError, UnspecifiedEnumValueError from ...metrics import Bounds, Metric -from ...types import InvalidLifetime, InvalidLifetimeError, Lifetime from .. import MicrogridId +from .._lifetime import InvalidLifetime, InvalidLifetimeError, Lifetime from ._ids import ElectricalComponentId @@ -39,8 +39,7 @@ class ElectricalComponent: # pylint: disable=too-many-instance-attributes ) """The operational lifetime of this electrical component. - An [`InvalidLifetime`][frequenz.client.common.types.InvalidLifetime] preserves - malformed wire data. + An [`InvalidLifetime`][....InvalidLifetime] preserves malformed wire data. Tip: Prefer [`get_operational_lifetime()`][..get_operational_lifetime] when diff --git a/src/frequenz/client/common/microgrid/electrical_components/_electrical_component_connection.py b/src/frequenz/client/common/microgrid/electrical_components/_electrical_component_connection.py index 5f28f753..94b012ef 100644 --- a/src/frequenz/client/common/microgrid/electrical_components/_electrical_component_connection.py +++ b/src/frequenz/client/common/microgrid/electrical_components/_electrical_component_connection.py @@ -7,7 +7,7 @@ from datetime import datetime, timezone from typing import Any, Self, assert_never -from ...types import InvalidLifetime, InvalidLifetimeError, Lifetime +from .._lifetime import InvalidLifetime, InvalidLifetimeError, Lifetime from ._ids import ElectricalComponentId @@ -60,8 +60,7 @@ class BaseElectricalComponentConnection: ) """The operational lifetime of the connection. - An [`InvalidLifetime`][frequenz.client.common.types.InvalidLifetime] preserves - malformed wire data. + An [`InvalidLifetime`][....InvalidLifetime] preserves malformed wire data. Tip: Prefer [`get_operational_lifetime()`][..get_operational_lifetime] when diff --git a/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component.py b/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component.py index 04460d0b..4a169f1a 100644 --- a/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component.py +++ b/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component.py @@ -16,9 +16,9 @@ from .....metrics import Bounds, Metric from .....metrics.proto.v1alpha8 import bounds_from_proto from .....proto import enum_from_proto -from .....types import InvalidLifetime, Lifetime -from .....types.proto.v1alpha8 import lifetime_from_proto from ...._ids import MicrogridId +from ...._lifetime import InvalidLifetime, Lifetime +from ....proto.v1alpha8 import lifetime_from_proto from ..._battery import ( Battery, LiIonBattery, diff --git a/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component_connection.py b/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component_connection.py index e4ce94f8..2c3b6ea8 100644 --- a/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component_connection.py +++ b/src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component_connection.py @@ -9,8 +9,8 @@ electrical_components_pb2, ) -from .....types import InvalidLifetime, Lifetime -from .....types.proto.v1alpha8 import lifetime_from_proto +from ...._lifetime import InvalidLifetime, Lifetime +from ....proto.v1alpha8 import lifetime_from_proto from ... import ( ElectricalComponentConnection, ElectricalComponentConnectionTypes, diff --git a/src/frequenz/client/common/microgrid/proto/v1alpha8/__init__.py b/src/frequenz/client/common/microgrid/proto/v1alpha8/__init__.py index 9c55d6c8..e9bc0dd4 100644 --- a/src/frequenz/client/common/microgrid/proto/v1alpha8/__init__.py +++ b/src/frequenz/client/common/microgrid/proto/v1alpha8/__init__.py @@ -3,8 +3,10 @@ """Conversion of microgrid objects from/to protobuf v1alpha8.""" +from ._lifetime import lifetime_from_proto from ._microgrid import microgrid_from_proto __all__ = [ + "lifetime_from_proto", "microgrid_from_proto", ] diff --git a/src/frequenz/client/common/types/proto/v1alpha8/_lifetime.py b/src/frequenz/client/common/microgrid/proto/v1alpha8/_lifetime.py similarity index 100% rename from src/frequenz/client/common/types/proto/v1alpha8/_lifetime.py rename to src/frequenz/client/common/microgrid/proto/v1alpha8/_lifetime.py diff --git a/src/frequenz/client/common/types/__init__.py b/src/frequenz/client/common/types/__init__.py index 18a52873..0e24665c 100644 --- a/src/frequenz/client/common/types/__init__.py +++ b/src/frequenz/client/common/types/__init__.py @@ -3,12 +3,6 @@ """Common types.""" -from ._lifetime import ( - BaseLifetime, - InvalidLifetime, - InvalidLifetimeError, - Lifetime, -) from ._location import ( InvalidCountryCode, InvalidCountryCodeError, @@ -20,15 +14,11 @@ ) __all__ = [ - "BaseLifetime", "InvalidCountryCode", "InvalidCountryCodeError", "InvalidLatitude", "InvalidLatitudeError", - "InvalidLifetime", - "InvalidLifetimeError", "InvalidLongitude", "InvalidLongitudeError", - "Lifetime", "Location", ] diff --git a/src/frequenz/client/common/types/proto/v1alpha8/__init__.py b/src/frequenz/client/common/types/proto/v1alpha8/__init__.py index f8f2881d..79b73356 100644 --- a/src/frequenz/client/common/types/proto/v1alpha8/__init__.py +++ b/src/frequenz/client/common/types/proto/v1alpha8/__init__.py @@ -3,10 +3,8 @@ """Common type protobuf v1alpha8 conversions.""" -from ._lifetime import lifetime_from_proto from ._location import location_from_proto __all__ = [ - "lifetime_from_proto", "location_from_proto", ] diff --git a/tests/types/_lifetime/__init__.py b/tests/microgrid/_lifetime/__init__.py similarity index 100% rename from tests/types/_lifetime/__init__.py rename to tests/microgrid/_lifetime/__init__.py diff --git a/tests/types/_lifetime/conftest.py b/tests/microgrid/_lifetime/conftest.py similarity index 100% rename from tests/types/_lifetime/conftest.py rename to tests/microgrid/_lifetime/conftest.py diff --git a/tests/types/_lifetime/test_base_lifetime.py b/tests/microgrid/_lifetime/test_base_lifetime.py similarity index 85% rename from tests/types/_lifetime/test_base_lifetime.py rename to tests/microgrid/_lifetime/test_base_lifetime.py index f81627db..7b633c86 100644 --- a/tests/types/_lifetime/test_base_lifetime.py +++ b/tests/microgrid/_lifetime/test_base_lifetime.py @@ -5,7 +5,7 @@ import pytest -from frequenz.client.common.types import BaseLifetime +from frequenz.client.common.microgrid import BaseLifetime def test_cannot_be_instantiated_directly() -> None: diff --git a/tests/types/_lifetime/test_invalid_lifetime.py b/tests/microgrid/_lifetime/test_invalid_lifetime.py similarity index 88% rename from tests/types/_lifetime/test_invalid_lifetime.py rename to tests/microgrid/_lifetime/test_invalid_lifetime.py index 74e385c5..132f9123 100644 --- a/tests/types/_lifetime/test_invalid_lifetime.py +++ b/tests/microgrid/_lifetime/test_invalid_lifetime.py @@ -5,7 +5,7 @@ from datetime import datetime -from frequenz.client.common.types import BaseLifetime, InvalidLifetime +from frequenz.client.common.microgrid import BaseLifetime, InvalidLifetime def test_is_base_lifetime_subclass() -> None: diff --git a/tests/types/_lifetime/test_invalid_lifetime_error.py b/tests/microgrid/_lifetime/test_invalid_lifetime_error.py similarity index 94% rename from tests/types/_lifetime/test_invalid_lifetime_error.py rename to tests/microgrid/_lifetime/test_invalid_lifetime_error.py index 29de2b52..e8e56f75 100644 --- a/tests/types/_lifetime/test_invalid_lifetime_error.py +++ b/tests/microgrid/_lifetime/test_invalid_lifetime_error.py @@ -6,7 +6,7 @@ from datetime import datetime from frequenz.client.common import InvalidAttributeError -from frequenz.client.common.types import InvalidLifetime, InvalidLifetimeError +from frequenz.client.common.microgrid import InvalidLifetime, InvalidLifetimeError def test_default_message(present: datetime, future: datetime) -> None: diff --git a/tests/types/_lifetime/test_lifetime.py b/tests/microgrid/_lifetime/test_lifetime.py similarity index 99% rename from tests/types/_lifetime/test_lifetime.py rename to tests/microgrid/_lifetime/test_lifetime.py index 4a70d1ac..7480bf9f 100644 --- a/tests/types/_lifetime/test_lifetime.py +++ b/tests/microgrid/_lifetime/test_lifetime.py @@ -9,7 +9,7 @@ import pytest -from frequenz.client.common.types import BaseLifetime, Lifetime +from frequenz.client.common.microgrid import BaseLifetime, Lifetime class _Time(Enum): diff --git a/tests/microgrid/electrical_components/proto/v1alpha8/conftest.py b/tests/microgrid/electrical_components/proto/v1alpha8/conftest.py index 81828306..1d611a6c 100644 --- a/tests/microgrid/electrical_components/proto/v1alpha8/conftest.py +++ b/tests/microgrid/electrical_components/proto/v1alpha8/conftest.py @@ -14,7 +14,7 @@ from google.protobuf.timestamp_pb2 import Timestamp from frequenz.client.common.metrics import Bounds, Metric -from frequenz.client.common.microgrid import MicrogridId +from frequenz.client.common.microgrid import Lifetime, MicrogridId from frequenz.client.common.microgrid.electrical_components import ( ElectricalComponent, ElectricalComponentCategory, @@ -24,7 +24,6 @@ _ElectricalComponentBaseData, ) from frequenz.client.common.proto import datetime_to_proto -from frequenz.client.common.types import Lifetime DEFAULT_LIFETIME = Lifetime( start_time=datetime(2020, 1, 1, tzinfo=timezone.utc), diff --git a/tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_base.py b/tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_base.py index 2f8fa301..2f9a6a3c 100644 --- a/tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_base.py +++ b/tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_base.py @@ -13,6 +13,7 @@ from google.protobuf.timestamp_pb2 import Timestamp from frequenz.client.common.metrics import Bounds, Metric +from frequenz.client.common.microgrid import InvalidLifetime, Lifetime from frequenz.client.common.microgrid.electrical_components import ( ElectricalComponentCategory, ) @@ -22,7 +23,6 @@ _metric_config_bounds_from_proto, _operational_mode_to_bools, ) -from frequenz.client.common.types import InvalidLifetime, Lifetime from .conftest import base_data_as_proto diff --git a/tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_connection.py b/tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_connection.py index d191facb..7f4b2315 100644 --- a/tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_connection.py +++ b/tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_connection.py @@ -15,6 +15,7 @@ ) from google.protobuf import timestamp_pb2 +from frequenz.client.common.microgrid import InvalidLifetime, Lifetime from frequenz.client.common.microgrid.electrical_components import ( ElectricalComponentConnection, ElectricalComponentId, @@ -24,7 +25,6 @@ electrical_component_connection_from_proto, electrical_component_connection_from_proto_with_issues, ) -from frequenz.client.common.types import InvalidLifetime, Lifetime @pytest.mark.parametrize( diff --git a/tests/microgrid/electrical_components/test_electrical_component_base.py b/tests/microgrid/electrical_components/test_electrical_component_base.py index cb5dbe0e..8fed6c64 100644 --- a/tests/microgrid/electrical_components/test_electrical_component_base.py +++ b/tests/microgrid/electrical_components/test_electrical_component_base.py @@ -10,15 +10,15 @@ from frequenz.client.common import UnrecognizedEnumValueError, UnspecifiedEnumValueError from frequenz.client.common.metrics import Bounds, Metric -from frequenz.client.common.microgrid import MicrogridId -from frequenz.client.common.microgrid.electrical_components import ( - ElectricalComponent, - ElectricalComponentId, -) -from frequenz.client.common.types import ( +from frequenz.client.common.microgrid import ( InvalidLifetime, InvalidLifetimeError, Lifetime, + MicrogridId, +) +from frequenz.client.common.microgrid.electrical_components import ( + ElectricalComponent, + ElectricalComponentId, ) diff --git a/tests/microgrid/electrical_components/test_electrical_component_connection.py b/tests/microgrid/electrical_components/test_electrical_component_connection.py index eefeccc0..34f05d87 100644 --- a/tests/microgrid/electrical_components/test_electrical_component_connection.py +++ b/tests/microgrid/electrical_components/test_electrical_component_connection.py @@ -8,16 +8,16 @@ import pytest +from frequenz.client.common.microgrid import ( + InvalidLifetime, + InvalidLifetimeError, + Lifetime, +) from frequenz.client.common.microgrid.electrical_components import ( BaseElectricalComponentConnection, ElectricalComponentConnection, ElectricalComponentId, ) -from frequenz.client.common.types import ( - InvalidLifetime, - InvalidLifetimeError, - Lifetime, -) def _make_connection( diff --git a/tests/microgrid/electrical_components/test_problematic_connection.py b/tests/microgrid/electrical_components/test_problematic_connection.py index 74c6a015..d10bd2b7 100644 --- a/tests/microgrid/electrical_components/test_problematic_connection.py +++ b/tests/microgrid/electrical_components/test_problematic_connection.py @@ -7,6 +7,7 @@ import pytest +from frequenz.client.common.microgrid import Lifetime from frequenz.client.common.microgrid.electrical_components import ( BaseElectricalComponentConnection, ElectricalComponentConnection, @@ -14,7 +15,6 @@ ProblematicElectricalComponentConnection, SelfReferencingElectricalComponentConnection, ) -from frequenz.client.common.types import Lifetime def test_abstract_problematic_connection_cannot_be_instantiated() -> None: diff --git a/tests/types/proto/v1alpha8/test_lifetime.py b/tests/microgrid/proto/v1alpha8/test_lifetime.py similarity index 95% rename from tests/types/proto/v1alpha8/test_lifetime.py rename to tests/microgrid/proto/v1alpha8/test_lifetime.py index 6779cbd3..c901b657 100644 --- a/tests/types/proto/v1alpha8/test_lifetime.py +++ b/tests/microgrid/proto/v1alpha8/test_lifetime.py @@ -11,8 +11,8 @@ from frequenz.api.common.v1alpha8.microgrid import lifetime_pb2 from google.protobuf import timestamp_pb2 -from frequenz.client.common.types import InvalidLifetime -from frequenz.client.common.types.proto.v1alpha8 import lifetime_from_proto +from frequenz.client.common.microgrid import InvalidLifetime +from frequenz.client.common.microgrid.proto.v1alpha8 import lifetime_from_proto @dataclass(frozen=True, kw_only=True) From 496edc626b7242db3f2366e302a3fce4dd427c93 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Thu, 16 Jul 2026 10:44:07 +0000 Subject: [PATCH 11/12] Add `__str__` to `Lifetime` and `InvalidLifetime` Adopt the `` marker convention. `Lifetime` renders as a half-open interval `(start,end]`; `None` endpoints become `-inf`/`+inf`. `InvalidLifetime` wraps the same rendering in an explicit `` marker to flag the relational invariant (`start <= end`) violation. Signed-off-by: Leandro Lucarella --- .../client/common/microgrid/_lifetime.py | 16 +++++ .../_lifetime/test_invalid_lifetime.py | 60 ++++++++++++++++++- tests/microgrid/_lifetime/test_lifetime.py | 55 ++++++++++++++++- 3 files changed, 129 insertions(+), 2 deletions(-) diff --git a/src/frequenz/client/common/microgrid/_lifetime.py b/src/frequenz/client/common/microgrid/_lifetime.py index db35fad7..e28c5736 100644 --- a/src/frequenz/client/common/microgrid/_lifetime.py +++ b/src/frequenz/client/common/microgrid/_lifetime.py @@ -71,6 +71,14 @@ def __post_init__(self) -> None: f"({self.end_time})" ) + def __str__(self) -> str: + """Return a compact string representation of this lifetime.""" + start_str = ( + self.start_time.isoformat() if self.start_time is not None else "-inf" + ) + end_str = self.end_time.isoformat() if self.end_time is not None else "+inf" + return f"({start_str},{end_str}]" + def is_operational_at(self, timestamp: datetime) -> bool: """Check whether this lifetime is active at a specific timestamp.""" # Handle start time - it's not active if start_time is in the future @@ -99,6 +107,14 @@ class InvalidLifetime(BaseLifetime): to receive a clear [`InvalidLifetimeError`][..InvalidLifetimeError]. """ + def __str__(self) -> str: + """Return a compact string representation of this invalid lifetime.""" + start_str = ( + self.start_time.isoformat() if self.start_time is not None else "-inf" + ) + end_str = self.end_time.isoformat() if self.end_time is not None else "+inf" + return f"" + class InvalidLifetimeError(InvalidAttributeError): """Raised when a semantic accessor sees an invalid lifetime. diff --git a/tests/microgrid/_lifetime/test_invalid_lifetime.py b/tests/microgrid/_lifetime/test_invalid_lifetime.py index 132f9123..1b3ffc3e 100644 --- a/tests/microgrid/_lifetime/test_invalid_lifetime.py +++ b/tests/microgrid/_lifetime/test_invalid_lifetime.py @@ -3,11 +3,31 @@ """Tests for `InvalidLifetime`.""" -from datetime import datetime +from dataclasses import dataclass +from datetime import datetime, timezone + +import pytest from frequenz.client.common.microgrid import BaseLifetime, InvalidLifetime +@dataclass(frozen=True, kw_only=True) +class _StrTestCase: + """Test case for `InvalidLifetime.__str__`.""" + + name: str + """The description of the test case.""" + + start_time: datetime | None + """The start time to use for the invalid lifetime.""" + + end_time: datetime | None + """The end time to use for the invalid lifetime.""" + + expected_str: str + """The expected string representation.""" + + def test_is_base_lifetime_subclass() -> None: """`InvalidLifetime` is a subclass of `BaseLifetime`.""" assert issubclass(InvalidLifetime, BaseLifetime) @@ -19,3 +39,41 @@ def test_accepts_invalid_range(present: datetime, future: datetime) -> None: assert lifetime.start_time is future assert lifetime.end_time is present + + +@pytest.mark.parametrize( + "case", + [ + _StrTestCase( + name="invalid_range", + start_time=datetime(2025, 6, 1, 15, 30, 45, tzinfo=timezone.utc), + end_time=datetime(2025, 1, 1, 12, 0, 0, tzinfo=timezone.utc), + expected_str=( + "" + ), + ), + _StrTestCase( + name="only_start", + start_time=datetime(2025, 6, 1, 15, 30, 45, tzinfo=timezone.utc), + end_time=None, + expected_str="", + ), + _StrTestCase( + name="only_end", + start_time=None, + end_time=datetime(2025, 1, 1, 12, 0, 0, tzinfo=timezone.utc), + expected_str="", + ), + _StrTestCase( + name="unbounded", + start_time=None, + end_time=None, + expected_str="", + ), + ], + ids=lambda case: case.name, +) +def test_str(case: _StrTestCase) -> None: + """`InvalidLifetime.__str__` wraps timestamps in the `` marker.""" + lifetime = InvalidLifetime(start_time=case.start_time, end_time=case.end_time) + assert str(lifetime) == case.expected_str diff --git a/tests/microgrid/_lifetime/test_lifetime.py b/tests/microgrid/_lifetime/test_lifetime.py index 7480bf9f..f6757570 100644 --- a/tests/microgrid/_lifetime/test_lifetime.py +++ b/tests/microgrid/_lifetime/test_lifetime.py @@ -4,7 +4,7 @@ """Tests for `Lifetime`.""" from dataclasses import dataclass -from datetime import datetime +from datetime import datetime, timezone from enum import Enum, auto import pytest @@ -79,6 +79,23 @@ class _FixedLifetimeTestCase: """The expected operational state.""" +@dataclass(frozen=True, kw_only=True) +class _StrTestCase: + """Test case for `Lifetime.__str__`.""" + + name: str + """The description of the test case.""" + + start_time: datetime | None + """The start time to use for the lifetime.""" + + end_time: datetime | None + """The end time to use for the lifetime.""" + + expected_str: str + """The expected string representation.""" + + def test_is_base_lifetime_subclass() -> None: """`Lifetime` is a subclass of `BaseLifetime`.""" assert issubclass(Lifetime, BaseLifetime) @@ -306,3 +323,39 @@ def test_active_at_with_fixed_lifetime( }[case.test_time] assert lifetime.is_operational_at(test_time) == case.expected_operational + + +@pytest.mark.parametrize( + "case", + [ + _StrTestCase( + name="full", + start_time=datetime(2025, 1, 1, 12, 0, 0, tzinfo=timezone.utc), + end_time=datetime(2025, 6, 1, 15, 30, 45, tzinfo=timezone.utc), + expected_str="(2025-01-01T12:00:00+00:00,2025-06-01T15:30:45+00:00]", + ), + _StrTestCase( + name="only_start", + start_time=datetime(2025, 1, 1, 12, 0, 0, tzinfo=timezone.utc), + end_time=None, + expected_str="(2025-01-01T12:00:00+00:00,+inf]", + ), + _StrTestCase( + name="only_end", + start_time=None, + end_time=datetime(2025, 6, 1, 15, 30, 45, tzinfo=timezone.utc), + expected_str="(-inf,2025-06-01T15:30:45+00:00]", + ), + _StrTestCase( + name="unbounded", + start_time=None, + end_time=None, + expected_str="(-inf,+inf]", + ), + ], + ids=lambda case: case.name, +) +def test_str(case: _StrTestCase) -> None: + """`Lifetime.__str__` renders ISO 8601 timestamps with `-inf`/`+inf` for `None`.""" + lifetime = Lifetime(start_time=case.start_time, end_time=case.end_time) + assert str(lifetime) == case.expected_str From 4751d9f24758a7bb9204fab824c956d9a762aa93 Mon Sep 17 00:00:00 2001 From: Leandro Lucarella Date: Tue, 14 Jul 2026 10:29:33 +0200 Subject: [PATCH 12/12] Update release notes Signed-off-by: Leandro Lucarella --- RELEASE_NOTES.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 9ad015fb..b7b191eb 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -90,7 +90,7 @@ * Added `frequenz.client.common.grid.proto.v1alpha8.delivery_area_from_proto2` returning `DeliveryArea | InvalidDeliveryArea`. This is the replacement for the now-deprecated `delivery_area_from_proto`. -* Added a new `frequenz.client.common.types.Lifetime` type together with the `frequenz.client.common.types.proto.v1alpha8.lifetime_from_proto` conversion function. +* Added a new `frequenz.client.common.microgrid.Lifetime` type together with the `frequenz.client.common.microgrid.proto.v1alpha8.lifetime_from_proto` conversion function. * Added a new `frequenz.client.common.types.Location` type together with the `frequenz.client.common.types.proto.v1alpha8.location_from_proto` conversion function.