Improve `Lifetime` validation and error handling by llucax · Pull Request #249 · frequenz-floss/frequenz-client-common-python

llucax · 2026-07-14T09:27:55Z

We now also allow storing invalid values, but encoded via a new type InvalidLifetime. This way users still can't read invalid values accidentally, but still allows users to inspect invalid values if they need to, instead of only reporting via logging.

Add new safe accesors for ElectricalComponent and ElectricalComponentConnection lifetime attributes.

Part of #239 and #251.

Copilot

Pull request overview

This PR refines how microgrid “lifetime” values are represented and consumed across the client-common wrapper layer, aiming to preserve malformed wire data safely while providing explicit failure modes via semantic accessors (aligning with the direction in #239 to move away from fragile “issues as strings” patterns).

Changes:

Introduce BaseLifetime, Lifetime, InvalidLifetime, and InvalidLifetimeError under frequenz.client.common.microgrid, and update lifetime conversion to preserve malformed ranges as InvalidLifetime.
Add safe lifetime accessors (get_operational_lifetime()) for ElectricalComponent and ElectricalComponentConnection, and make operational checks raise InvalidLifetimeError when malformed data is present.
Update proto converters, tests, and release notes to reflect the new lifetime handling and imports (including moving lifetime conversion out of frequenz.client.common.types.*).

Reviewed changes

Copilot reviewed 35 out of 35 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
tests/microgrid/proto/v1alpha8/test_lifetime.py	Updates lifetime proto tests to expect `InvalidLifetime` preservation instead of raising.
tests/microgrid/electrical_components/test_simple_components.py	Adjusts component construction expectations (adds `model`).
tests/microgrid/electrical_components/test_problematic.py	Adjusts problematic component tests for new `model` requirement.
tests/microgrid/electrical_components/test_problematic_connection.py	Updates `Lifetime` import to microgrid module.
tests/microgrid/electrical_components/test_power_transformer.py	Updates construction to include `model`.
tests/microgrid/electrical_components/test_inverter.py	Updates construction to include `model`.
tests/microgrid/electrical_components/test_grid_connection_point.py	Updates construction to include `model`.
tests/microgrid/electrical_components/test_ev_charger.py	Updates construction to include `model`.
tests/microgrid/electrical_components/test_electrical_component_connection.py	Adds tests for safe lifetime accessors and invalid-lifetime raising behavior.
tests/microgrid/electrical_components/test_electrical_component_base.py	Adds component-level safe lifetime accessor tests and updates `model` expectations.
tests/microgrid/electrical_components/test_battery.py	Updates construction to include `model`.
tests/microgrid/electrical_components/proto/v1alpha8/test_raw_storage.py	Ensures model is preserved in raw/unrecognized component conversion cases.
tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_connection.py	Updates connection proto conversion expectations: unbounded default and invalid lifetime preservation.
tests/microgrid/electrical_components/proto/v1alpha8/test_electrical_component_base.py	Updates base-data proto conversion expectations for lifetime handling and minor issues.
tests/microgrid/electrical_components/proto/v1alpha8/test_class.py	Updates test data dict to include `model`.
tests/microgrid/electrical_components/proto/v1alpha8/conftest.py	Moves `Lifetime` import to microgrid module and sets default lifetime fixture.
tests/microgrid/_lifetime/test_lifetime.py	Adjusts lifetime tests to new lifetime type hierarchy (`BaseLifetime`).
tests/microgrid/_lifetime/test_invalid_lifetime.py	Adds tests for `InvalidLifetime` preserving malformed ranges.
tests/microgrid/_lifetime/test_invalid_lifetime_error.py	Adds tests for `InvalidLifetimeError` behavior and inheritance.
tests/microgrid/_lifetime/test_base_lifetime.py	Adds tests ensuring `BaseLifetime` cannot be instantiated directly.
tests/microgrid/_lifetime/conftest.py	Adds shared time fixtures for lifetime-related tests.
tests/microgrid/_lifetime/init.py	Introduces package init for lifetime tests.
src/frequenz/client/common/types/proto/v1alpha8/_lifetime.py	Removes the old lifetime proto converter from `types`.
src/frequenz/client/common/types/proto/v1alpha8/init.py	Stops exporting `lifetime_from_proto` from `types.proto.v1alpha8`.
src/frequenz/client/common/types/_lifetime.py	Removes the old `types.Lifetime` implementation.
src/frequenz/client/common/types/init.py	Stops exporting `Lifetime` from `frequenz.client.common.types`.
src/frequenz/client/common/microgrid/proto/v1alpha8/_lifetime.py	Adds microgrid-scoped lifetime proto conversion that returns `Lifetime
src/frequenz/client/common/microgrid/proto/v1alpha8/init.py	Exports `lifetime_from_proto` from microgrid proto package.
src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component.py	Updates component proto parsing to use new lifetime types and preserve malformed lifetimes.
src/frequenz/client/common/microgrid/electrical_components/proto/v1alpha8/_electrical_component_connection.py	Updates connection proto parsing to use new lifetime types; drops minor-issues collection for lifetime.
src/frequenz/client/common/microgrid/electrical_components/_electrical_component.py	Adds `get_operational_lifetime()` and makes operational checks raise on invalid lifetime.
src/frequenz/client/common/microgrid/electrical_components/_electrical_component_connection.py	Adds `get_operational_lifetime()` and makes operational checks raise on invalid lifetime.
src/frequenz/client/common/microgrid/_lifetime.py	Introduces `BaseLifetime`, `Lifetime`, `InvalidLifetime`, `InvalidLifetimeError`.
src/frequenz/client/common/microgrid/init.py	Re-exports new lifetime types at `frequenz.client.common.microgrid.*`.
RELEASE_NOTES.md	Updates release notes entry related to lifetime types/converters.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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 <luca-frequenz@llucax.com>

This class will be used as the base for both valid and invalid lifetime classes. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

`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 <luca-frequenz@llucax.com>

`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 <luca-frequenz@llucax.com>

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 <luca-frequenz@llucax.com>

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 <luca-frequenz@llucax.com>

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 <luca-frequenz@llucax.com>

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 <luca-frequenz@llucax.com>

`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_<thing>.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 <luca-frequenz@llucax.com>

This is to match where `Lifetime` is defined in the protobuf messages. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

Adopt the `<invalid:VALUE>` marker convention. `Lifetime` renders as a half-open interval `(start,end]`; `None` endpoints become `-inf`/`+inf`. `InvalidLifetime` wraps the same rendering in an explicit `<invalid:...>` marker to flag the relational invariant (`start <= end`) violation. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

llucax · 2026-07-16T10:54:52Z

Maybe you can give this a look too @cwasicki ? 😇

cwasicki

LGTM

cwasicki · 2026-07-16T17:12:51Z

+@pytest.fixture
+def present() -> datetime:
+    """Provide the current UTC time."""
+    return datetime.now(timezone.utc)


Is now mocked or do tests depend on time?

They are not mocked nor they depend on time, sort of. It is not mocked but we create past and future relative to present, so it works, but we should mock it, yeah, that was a dumb idea. But not for this PR. :)

llucax requested a review from a team as a code owner July 14, 2026 09:27

llucax requested review from shsms and removed request for a team July 14, 2026 09:27

llucax self-assigned this Jul 14, 2026

llucax added this to the v0.4.1 milestone Jul 14, 2026

llucax requested review from Copilot and tiyash-basu-frequenz July 14, 2026 09:29

Copilot started reviewing on behalf of llucax July 14, 2026 09:36 View session

Copilot AI reviewed Jul 14, 2026

View reviewed changes

llucax force-pushed the err-lifetime branch 2 times, most recently from 032c422 to d959051 Compare July 14, 2026 11:36

llucax enabled auto-merge July 14, 2026 11:36

This was referenced Jul 15, 2026

Standardize <invalid:VALUE> across __str__ #251

Open

Improve Location validation and error handling #248

Merged

llucax added 10 commits July 16, 2026 12:49

Make ElectricalComponent.model required

262bbe2

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 <luca-frequenz@llucax.com>

Add BaseLifetime abstract base class

7d3d651

This class will be used as the base for both valid and invalid lifetime classes. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

Make Lifetime inherit from BaseLifetime

10571ed

`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 <luca-frequenz@llucax.com>

Stop reporting lifetime issues via strings

dc6a541

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 <luca-frequenz@llucax.com>

Move Lifetime from types to microgrid

a985835

This is to match where `Lifetime` is defined in the protobuf messages. Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

llucax force-pushed the err-lifetime branch from d959051 to 67058e0 Compare July 16, 2026 10:53

Update release notes

4751d9f

Signed-off-by: Leandro Lucarella <luca-frequenz@llucax.com>

llucax force-pushed the err-lifetime branch from 67058e0 to 4751d9f Compare July 16, 2026 10:54

llucax requested a review from cwasicki July 16, 2026 10:54

cwasicki reviewed Jul 16, 2026

View reviewed changes

cwasicki approved these changes Jul 17, 2026

View reviewed changes

llucax added this pull request to the merge queue Jul 17, 2026

Merged via the queue into frequenz-floss:v0.x.x with commit 3409422 Jul 17, 2026
9 checks passed

llucax deleted the err-lifetime branch July 17, 2026 09:51

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Improve `Lifetime` validation and error handling#249

Improve `Lifetime` validation and error handling#249
llucax merged 12 commits into
frequenz-floss:v0.x.xfrom
llucax:err-lifetime

llucax commented Jul 14, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

llucax commented Jul 16, 2026

Uh oh!

cwasicki left a comment

Uh oh!

cwasicki Jul 16, 2026

Uh oh!

llucax Jul 17, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

llucax commented Jul 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

llucax commented Jul 16, 2026

Uh oh!

cwasicki left a comment

Choose a reason for hiding this comment

Uh oh!

cwasicki Jul 16, 2026

Choose a reason for hiding this comment

Uh oh!

llucax Jul 17, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

llucax commented Jul 14, 2026 •

edited

Loading