diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 44b3c0c0..b022dc72 100644 --- a/RELEASE_NOTES.md +++ b/RELEASE_NOTES.md @@ -91,6 +91,16 @@ Migrate to `bounds_set` (or `get_bounds_set()`) for all of these. +* `frequenz.client.common.proto.datetime_from_proto` is now deprecated; use `datetime_from_proto2` instead. + +* `frequenz.client.common.metrics.MetricSample.sample_time` is now a deprecated read-only property; use the new `sample_time2` field instead. + + The field became `datetime | InvalidDatetime`, which the released `datetime` annotation cannot express, so it was renamed. Reading `sample_time` still returns a `datetime` and now emits a `DeprecationWarning`; for a malformed wire timestamp it raises `InvalidDatetimeError` (a `ValueError`) rather than returning a repaired value. `get_sample_time()` does the same without the warning. + + Constructing with `sample_time=` is **not** deprecated and keeps working: it accepts a well-formed `datetime` today and will accept the wider type once `sample_time2` is renamed back to `sample_time`. Use `sample_time2=` to build a sample from a malformed wire timestamp. + + Because `sample_time` is no longer a real field, `dataclasses.fields()`, `asdict()`, `astuple()` and `replace()` see `sample_time2`. + * `frequenz.client.common.metrics.proto.v1alpha8.metric_sample_from_proto_with_issues` no longer drops invalid bounds or reports them as a major issue. Malformed bounds are now preserved in the returned `MetricSample.bounds_set` as an `InvalidBoundsSet` (validity is encoded in the type), so the previous "bounds for ... is invalid, ignoring these bounds" major issue is no longer produced. @@ -136,8 +146,15 @@ * `frequenz.client.common.metrics.MetricConnection.get_category()` * `frequenz.client.common.metrics.MetricSample.get_metric()` * `frequenz.client.common.metrics.MetricSample.get_bounds_set()` + * `frequenz.client.common.metrics.MetricSample.get_sample_time()` * `frequenz.client.common.microgrid.electrical_components.ElectricalComponent.get_metric_config_bounds()` +* Added `frequenz.client.common.InvalidDatetime`, a protobuf-independent wrapper preserving the raw `seconds` and `nanos` of a wire timestamp that is not a well-formed protobuf `Timestamp`, and `frequenz.client.common.InvalidDatetimeError`, raised by the safe accessors for those fields. Both are exported from the top-level package, not from `frequenz.client.common.types`, because a timestamp is not a `frequenz-api-common` message. See the Upgrading section for the fields that can now hold one. + +* Added `frequenz.client.common.metrics.MetricSample.sample_time2`, typed `datetime | InvalidDatetime`, replacing the now-deprecated `sample_time` property (see Upgrading). + +* Added `frequenz.client.common.proto.datetime_from_proto2` returning `datetime | InvalidDatetime`. This is the replacement for the now-deprecated `datetime_from_proto`. + * Added new delivery-area class hierarchy: * `frequenz.client.common.grid.BaseDeliveryArea` — abstract common supertype of the two concrete leaves; not directly instantiable. diff --git a/docs/user-guide/overview.md b/docs/user-guide/overview.md index 270dc565..ed238732 100644 --- a/docs/user-guide/overview.md +++ b/docs/user-guide/overview.md @@ -4,6 +4,13 @@ The library groups its wrappers by the kind of common API data you receive. Use this map to find the relevant domain, then follow its links for the public API details. +## Utilities + +[`InvalidDatetime`][frequenz.client.common.InvalidDatetime] is not part of +`frequenz-api-common` so it lives in the top-level package. It preserves a wire +timestamp that is not well-formed and can appear in any wrapper where a +[`datetime`][datetime.datetime] is expected. + ## Grid Start with [`DeliveryArea`][frequenz.client.common.grid.DeliveryArea] and @@ -37,9 +44,8 @@ Use [`Microgrid`][frequenz.client.common.microgrid.Microgrid], [`MicrogridId`][frequenz.client.common.microgrid.MicrogridId], [`EnterpriseId`][frequenz.client.common.microgrid.EnterpriseId], and [`Lifetime`][frequenz.client.common.microgrid.Lifetime]. -[`BaseLifetime`][frequenz.client.common.microgrid.BaseLifetime] and -[`InvalidLifetime`][frequenz.client.common.microgrid.InvalidLifetime] represent -the lifetime variants, while +[`InvalidLifetime`][frequenz.client.common.microgrid.InvalidLifetime] carries +malformed wire data, while [`InvalidLifetimeError`][frequenz.client.common.microgrid.InvalidLifetimeError] is raised by safe accessors. diff --git a/docs/user-guide/reading-string-output.md b/docs/user-guide/reading-string-output.md index ff674825..8a9f4dbd 100644 --- a/docs/user-guide/reading-string-output.md +++ b/docs/user-guide/reading-string-output.md @@ -23,6 +23,12 @@ and [`InvalidCountryCode`][frequenz.client.common.types.InvalidCountryCode] use the same marker. See [validity in the type](validity-in-the-type.md) to handle these values. +[`InvalidDatetime`][frequenz.client.common.InvalidDatetime] prints both raw +numbers of a malformed wire timestamp inside the marker, as +``. The nanosecond part always carries its sign, +because a fraction outside `[0, 999999999]` is one of the two reasons the +timestamp is there at all. + An unexpected raw number without `` means something different: the data is well-formed, but this client version does not recognize it yet. For example, [`UnrecognizedElectricalComponent`][frequenz.client.common.microgrid.electrical_components.UnrecognizedElectricalComponent] diff --git a/docs/user-guide/validity-in-the-type.md b/docs/user-guide/validity-in-the-type.md index d037987f..5d8f37f4 100644 --- a/docs/user-guide/validity-in-the-type.md +++ b/docs/user-guide/validity-in-the-type.md @@ -66,6 +66,48 @@ latitude or raises [`InvalidLatitudeError`][frequenz.client.common.types.InvalidLatitudeError], whose [`InvalidLatitudeError.value`][frequenz.client.common.types.InvalidLatitudeError.value] is the raw value. +Timestamps work the same way. A protobuf `Timestamp` counts seconds and +nanoseconds as plain integers, so a message can carry values the `Timestamp` +contract does not allow: an instant outside the years 1 to 9999, or a +nanosecond fraction outside `[0, 999999999]`. Every wrapper timestamp field is +therefore `datetime | `[`InvalidDatetime`][frequenz.client.common.InvalidDatetime], +with [`InvalidDatetime.seconds`][frequenz.client.common.InvalidDatetime.seconds] +and [`InvalidDatetime.nanos`][frequenz.client.common.InvalidDatetime.nanos] +holding exactly what the server sent: + +```python +from datetime import datetime + +from frequenz.client.common.metrics import BoundsSet, Metric, MetricSample +from frequenz.client.common import InvalidDatetime, InvalidDatetimeError + +sample = MetricSample( + sample_time2=InvalidDatetime(seconds=253402300800, nanos=0), + metric=Metric.AC_POWER_ACTIVE, + value=42.0, + bounds_set=BoundsSet(), +) + +match sample.sample_time2: + case InvalidDatetime(seconds=raw_seconds): + print(raw_seconds) # 253402300800 + case datetime() as sample_time: + print(sample_time.isoformat()) + +try: + sample.get_sample_time() +except InvalidDatetimeError as error: + print(error.attr_name) # sample_time2 + print(error.datetime) # +``` + +A [`Lifetime`][frequenz.client.common.microgrid.Lifetime] is the exception: its +two ends are plain `datetime | None`, because a period whose ends cannot be +ordered is not a usable period. Only +[`InvalidLifetime`][frequenz.client.common.microgrid.InvalidLifetime] can hold +an [`InvalidDatetime`][frequenz.client.common.InvalidDatetime], and a malformed +wire timestamp gives you one of those instead. + Some wrapper types use a dedicated subclass for a value that is invalid, unspecified, or unrecognized. For a battery, that can be [`UnspecifiedBattery`][frequenz.client.common.microgrid.electrical_components.UnspecifiedBattery] diff --git a/docs/wrapping-guide/validity-in-the-type.md b/docs/wrapping-guide/validity-in-the-type.md index 0fe58953..ab8de2d6 100644 --- a/docs/wrapping-guide/validity-in-the-type.md +++ b/docs/wrapping-guide/validity-in-the-type.md @@ -18,6 +18,14 @@ For example, [`InvalidDeliveryArea`][frequenz.client.common.grid.InvalidDeliveryArea] share a base while making their validity visible in annotations. +Share a base only while both types hold the same field types. When the invalid +type has to accept a wider type in a field, because that field can itself +carry an `Invalid*` wrapper, write two independent classes instead. +[`BoundsSet`][frequenz.client.common.metrics.BoundsSet] and +[`InvalidBoundsSet`][frequenz.client.common.metrics.InvalidBoundsSet] do this, +as do [`Lifetime`][frequenz.client.common.microgrid.Lifetime] and +[`InvalidLifetime`][frequenz.client.common.microgrid.InvalidLifetime]. + Normal constructors enforce the valid subclass's rules. A conversion function that sees invalid protobuf data creates the matching invalid subclass and returns `X | InvalidX`. The invalid subclass keeps the raw fields for @@ -50,6 +58,17 @@ with [`InvalidLatitude`][frequenz.client.common.types.InvalidLatitude], [`InvalidCountryCode`][frequenz.client.common.types.InvalidCountryCode]. Each wrapper keeps the raw value while leaving the other fields usable. +Reuse an existing field wrapper when several types have the same field. Every +wrapper timestamp is `datetime | InvalidDatetime` because a protobuf +`Timestamp` can break its own contract no matter which message it arrives in. +One wrapper and one +[`InvalidDatetimeError`][frequenz.client.common.InvalidDatetimeError] mean a +caller learns the pattern once. A field wrapper is protobuf-independent, so it +belongs in a public type module, not next to the conversion function that +produces it. When, it wraps no `frequenz-api-common` message at all, like a +timestamp, put it directly in the top-level package or an utility-specific +module, do not mix it with a domain-specific module. + ## Represent protobuf recovery as a subtype When the class identifies a protobuf category or type, use dedicated subclasses diff --git a/src/frequenz/client/common/__init__.py b/src/frequenz/client/common/__init__.py index 0580c58d..e5e10304 100644 --- a/src/frequenz/client/common/__init__.py +++ b/src/frequenz/client/common/__init__.py @@ -3,6 +3,7 @@ """Common code and utilities for Frequenz API clients.""" +from ._datetime import InvalidDatetime, InvalidDatetimeError from ._exception import ( ClientCommonError, InvalidAttributeError, @@ -14,6 +15,8 @@ __all__ = [ "ClientCommonError", "InvalidAttributeError", + "InvalidDatetime", + "InvalidDatetimeError", "MissingFieldError", "UnrecognizedEnumValueError", "UnspecifiedEnumValueError", diff --git a/src/frequenz/client/common/_datetime.py b/src/frequenz/client/common/_datetime.py new file mode 100644 index 00000000..40f8e92b --- /dev/null +++ b/src/frequenz/client/common/_datetime.py @@ -0,0 +1,82 @@ +# License: MIT +# Copyright © 2026 Frequenz Energy-as-a-Service GmbH + +"""Timestamps that have no Python equivalent.""" + +from dataclasses import dataclass + +from ._exception import InvalidAttributeError + + +@dataclass(frozen=True, kw_only=True) +class InvalidDatetime: + """A wire timestamp with no [`datetime`][datetime.datetime] equivalent. + + A protobuf `Timestamp` counts whole [`seconds`][.seconds] since the Unix + epoch plus a fraction in [`nanos`][.nanos]. Both are plain integers on the + wire, so a decoded message can carry a value with no meaningful + [`datetime`][datetime.datetime]: + + - a [`seconds`][.seconds] count outside the years 1 to 9999, which is both + the range the protobuf specification allows and the range + [`datetime`][datetime.datetime] covers; + - a [`nanos`][.nanos] fraction outside `[0, 999999999]`, which the + protobuf specification does not allow and whose intended meaning is + therefore unknown. + + This wrapper keeps both raw numbers unchanged so callers can inspect, + report, or reinterpret what the server sent, instead of losing the message + to an exception or to a silently repaired value. It is + protobuf-independent, so it can appear in public wrapper fields. + """ + + seconds: int + """The raw number of seconds since 1970-01-01T00:00:00Z.""" + + nanos: int + """The raw fraction of a second, in nanoseconds, to add to [`seconds`][..seconds].""" + + def __str__(self) -> str: + """Return a compact representation flagging this as an invalid value.""" + return f"" + + +class InvalidDatetimeError(InvalidAttributeError): + """Raised when a semantic accessor sees a timestamp with no `datetime` equivalent. + + The offending [`InvalidDatetime`][..InvalidDatetime] is available as the + [`datetime`][.datetime] attribute so callers can inspect the raw wire + data. + + This is also a [`ValueError`][] for convenience. + """ + + def __init__( + self, + instance: object, + attr_name: str, + datetime: InvalidDatetime, + 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. + datetime: The invalid timestamp instance. + message: A custom error message. If `None`, a default message + mentioning the invalid timestamp is used. + """ + self.datetime: InvalidDatetime = datetime + """The invalid timestamp that caused this error.""" + + super().__init__( + instance, + attr_name, + ( + message + if message is not None + else f"invalid timestamp {datetime} for attribute {attr_name!r} " + f"in {instance}" + ), + ) diff --git a/src/frequenz/client/common/metrics/_sample.py b/src/frequenz/client/common/metrics/_sample.py index 79a38917..ea80c836 100644 --- a/src/frequenz/client/common/metrics/_sample.py +++ b/src/frequenz/client/common/metrics/_sample.py @@ -13,6 +13,7 @@ from frequenz.core.typing import FloatInt from typing_extensions import deprecated +from .._datetime import InvalidDatetime, InvalidDatetimeError from .._exception import UnrecognizedEnumValueError, UnspecifiedEnumValueError from ._bounds import Bounds, BoundsSet, InvalidBoundsSet, InvalidBoundsSetError from ._metric import Metric @@ -186,8 +187,22 @@ class MetricSample: to request current values within the bounds. """ - sample_time: datetime - """The moment when the metric was sampled.""" + sample_time2: datetime | InvalidDatetime + """The moment when the metric was sampled. + + A [`datetime`][datetime.datetime] for a well-formed wire timestamp, or an + [`InvalidDatetime`][....InvalidDatetime] preserving the raw seconds and + nanoseconds when the wire carried a malformed one. + + Tip: + Prefer [`get_sample_time()`][..get_sample_time] to obtain a valid + [`datetime`][datetime.datetime] or a clear error. + + Note: + This field replaces the deprecated [`sample_time`][..sample_time] + property, which cannot express the malformed case. It will be renamed + back to `sample_time` once that property is removed. + """ metric: Metric | int """The metric that was sampled. @@ -242,12 +257,14 @@ class MetricSample: sampled from is important. """ - # This custom `__init__` should be removed once the deprecated `bounds` field is removed. + # This custom `__init__` should be removed once the deprecated `bounds` and + # `sample_time` fields are removed. # pylint: disable-next=too-many-arguments - def __init__( + def __init__( # noqa: DOC502 self, *, - sample_time: datetime, + sample_time: datetime | None = None, + sample_time2: datetime | InvalidDatetime | None = None, metric: Metric | int, value: FloatInt | AggregatedMetricValue | None, bounds_set: BoundsSet | InvalidBoundsSet | None = None, @@ -257,7 +274,14 @@ def __init__( """Initialize this metric sample. Args: - sample_time: The moment when the metric was sampled. + sample_time: The moment when the metric was sampled, as a + well-formed [`datetime`][datetime.datetime]. This spelling + stays valid: it will accept the wider type once + [`sample_time2`][..sample_time2] is renamed back to + `sample_time`. + sample_time2: The moment when the metric was sampled, which may be + an [`InvalidDatetime`][....InvalidDatetime]. Use this to build + a sample from a malformed wire timestamp. metric: The metric that was sampled. value: The value of the sampled metric. bounds_set: The bounds that apply to the metric sample. @@ -268,7 +292,59 @@ def __init__( Raises: TypeError: If both `bounds_set` and the deprecated `bounds` are - given, or if neither is given. + given, or if neither is given; or if both `sample_time` and + `sample_time2` are given, or if neither is given. + """ + sample_time2 = self._resolve_sample_time(sample_time, sample_time2) + bounds_set = self._resolve_bounds_set(bounds_set, bounds) + object.__setattr__(self, "sample_time2", sample_time2) + object.__setattr__(self, "metric", metric) + object.__setattr__(self, "value", value) + object.__setattr__(self, "bounds_set", bounds_set) + object.__setattr__(self, "connection", connection) + + @staticmethod + def _resolve_sample_time( + sample_time: datetime | None, sample_time2: datetime | InvalidDatetime | None + ) -> datetime | InvalidDatetime: + """Pick the sample time from the current and the compatibility argument. + + Args: + sample_time: The compatibility argument. + sample_time2: The current argument. + + Returns: + The sample time to store. + + Raises: + TypeError: If both or neither argument is given. + """ + if sample_time is not None and sample_time2 is not None: + raise TypeError( + "`MetricSample` accepts either `sample_time` or `sample_time2`, " + "not both." + ) + if sample_time is not None: + return sample_time + if sample_time2 is None: + raise TypeError("`MetricSample` requires the `sample_time2` argument.") + return sample_time2 + + @staticmethod + def _resolve_bounds_set( + bounds_set: BoundsSet | InvalidBoundsSet | None, bounds: list[Bounds] | None + ) -> BoundsSet | InvalidBoundsSet: + """Pick the bounds set from the current and the deprecated argument. + + Args: + bounds_set: The current argument. + bounds: The deprecated argument. + + Returns: + The bounds set to store. + + Raises: + TypeError: If both or neither argument is given. """ if bounds is not None and bounds_set is not None: raise TypeError( @@ -279,16 +355,12 @@ def __init__( warnings.warn( "The `bounds` argument is deprecated; use `bounds_set` instead.", DeprecationWarning, - stacklevel=2, + stacklevel=4, ) - bounds_set = BoundsSet(bounds=tuple(bounds)) + return BoundsSet(bounds=tuple(bounds)) if bounds_set is None: raise TypeError("`MetricSample` requires the `bounds_set` argument.") - object.__setattr__(self, "sample_time", sample_time) - object.__setattr__(self, "metric", metric) - object.__setattr__(self, "value", value) - object.__setattr__(self, "bounds_set", bounds_set) - object.__setattr__(self, "connection", connection) + return bounds_set def __str__(self) -> str: """Return a compact string representation of this sample.""" @@ -308,12 +380,35 @@ def __str__(self) -> str: sample = f"{sample}@{self.connection}" return sample + @property + @deprecated("`MetricSample.sample_time` is deprecated; use `sample_time2` instead.") + def sample_time(self) -> datetime: # noqa: DOC502 + """The moment when the metric was sampled. + + Warning: Deprecated + Use [`sample_time2`][..sample_time2] instead, or + [`get_sample_time()`][..get_sample_time] when a valid + [`datetime`][datetime.datetime] is required. This property keeps + the released `datetime` type, so it cannot express a malformed wire + timestamp and raises for one instead. + + Returns: + The sample time, when it is a valid + [`datetime`][datetime.datetime]. + + Raises: + InvalidDatetimeError: If the sample time is an + [`InvalidDatetime`][....InvalidDatetime]. The offending + instance is available on the error's `datetime` attribute. + """ + return self.get_sample_time() + @property @deprecated("`MetricSample.bounds` is deprecated; use `bounds_set` instead.") def bounds(self) -> list[Bounds]: """The valid bounds that apply to the metric sample. - Deprecated: + Warning: Deprecated Use `bounds_set` instead. For backward compatibility this returns only the valid [`Bounds`][...Bounds] from `bounds_set` (dropping any malformed entries, as the old field did), but it returns the @@ -362,6 +457,30 @@ def as_single_value( case unexpected: assert_never(unexpected) + def get_sample_time(self) -> datetime: + """Return the sample time as a valid `datetime`. + + This is the higher-level accessor for the lower-level + [`sample_time2`][frequenz.client.common.metrics.MetricSample.sample_time2] + field: it returns a valid [`datetime`][datetime.datetime] or raises + instead of exposing an [`InvalidDatetime`][....InvalidDatetime]. + + Returns: + The sample time when it is a valid [`datetime`][datetime.datetime]. + + Raises: + InvalidDatetimeError: If the sample time is an + [`InvalidDatetime`][....InvalidDatetime]. The offending + instance is available on the error's `datetime` attribute. + """ + match self.sample_time2: + case datetime() as sample_time: + return sample_time + case InvalidDatetime() as invalid: + raise InvalidDatetimeError(self, "sample_time2", invalid) + case unexpected: + assert_never(unexpected) + def get_metric(self) -> Metric: """Return the sampled metric as a known enum member. diff --git a/src/frequenz/client/common/metrics/proto/v1alpha8/_sample.py b/src/frequenz/client/common/metrics/proto/v1alpha8/_sample.py index bb924083..706da037 100644 --- a/src/frequenz/client/common/metrics/proto/v1alpha8/_sample.py +++ b/src/frequenz/client/common/metrics/proto/v1alpha8/_sample.py @@ -8,7 +8,8 @@ from frequenz.api.common.v1alpha8.metrics import metrics_pb2 from typing_extensions import deprecated -from ....proto import datetime_from_proto +from ...._datetime import InvalidDatetime +from ....proto import datetime_from_proto2 from ..._metric import Metric from ..._sample import ( AggregatedMetricValue, @@ -75,8 +76,9 @@ def metric_sample_from_proto( Malformed or forward-incompatible input is surfaced through the returned type rather than a side channel: an unspecified metric is preserved as the raw integer `0` and an unrecognized one as its raw integer value in the - `metric` field (typed `Metric | int`), and malformed bounds as an - `InvalidBoundsSet` in `bounds_set`. + `metric` field (typed `Metric | int`), malformed bounds as an + `InvalidBoundsSet` in `bounds_set`, and a malformed sample time as an + `InvalidDatetime` in `sample_time2`. Args: message: The protobuf message to convert. @@ -84,7 +86,7 @@ def metric_sample_from_proto( Returns: The resulting [`MetricSample`][....MetricSample] object. """ - sample_time = datetime_from_proto(message.sample_time) + sample_time = datetime_from_proto2(message.sample_time) raw_metric = message.metric metric: Metric | int = ( @@ -108,7 +110,7 @@ def metric_sample_from_proto( connection = metric_connection_from_proto(message.connection) return MetricSample( - sample_time=sample_time, + sample_time2=sample_time, metric=metric, value=value, bounds_set=bounds_set, @@ -174,10 +176,17 @@ def metric_sample_from_proto_with_issues( Warning: Deprecated Use [`metric_sample_from_proto`][..metric_sample_from_proto] instead and inspect the returned type. The new converter encodes an - unspecified or unrecognized `metric` (`Metric | int`) and malformed - bounds (`InvalidBoundsSet`) in the returned `MetricSample` rather than + unspecified or unrecognized `metric` (`Metric | int`), malformed + bounds (`InvalidBoundsSet`) and an unrepresentable sample time + (`InvalidDatetime`) in the returned `MetricSample` rather than routing them through a side-channel string list. + Note: + A malformed `sample_time` still raises `ValueError`, as it did when the + conversion went through `datetime_from_proto`. Only + [`metric_sample_from_proto`][..metric_sample_from_proto] keeps it in + the returned sample. + Args: message: The protobuf message to convert. major_issues: A list to append major issues to. @@ -185,8 +194,14 @@ def metric_sample_from_proto_with_issues( Returns: The resulting [`MetricSample`][....MetricSample] object. + + Raises: + ValueError: If the sample time is not a well-formed protobuf + `Timestamp`. """ - sample_time = datetime_from_proto(message.sample_time) + sample_time = datetime_from_proto2(message.sample_time) + if isinstance(sample_time, InvalidDatetime): + raise ValueError(f"malformed sample_time {sample_time}") raw_metric = message.metric metric: Metric | int = ( @@ -214,7 +229,7 @@ def metric_sample_from_proto_with_issues( ) return MetricSample( - sample_time=sample_time, + sample_time2=sample_time, metric=metric, value=value, bounds_set=bounds_set, diff --git a/src/frequenz/client/common/microgrid/__init__.py b/src/frequenz/client/common/microgrid/__init__.py index ee982607..27580c4a 100644 --- a/src/frequenz/client/common/microgrid/__init__.py +++ b/src/frequenz/client/common/microgrid/__init__.py @@ -5,7 +5,6 @@ from ._ids import EnterpriseId, MicrogridId from ._lifetime import ( - BaseLifetime, InvalidLifetime, InvalidLifetimeError, Lifetime, @@ -13,7 +12,6 @@ from ._microgrid import Microgrid __all__ = [ - "BaseLifetime", "EnterpriseId", "InvalidLifetime", "InvalidLifetimeError", diff --git a/src/frequenz/client/common/microgrid/_lifetime.py b/src/frequenz/client/common/microgrid/_lifetime.py index c35bce31..7cc086b6 100644 --- a/src/frequenz/client/common/microgrid/_lifetime.py +++ b/src/frequenz/client/common/microgrid/_lifetime.py @@ -5,49 +5,25 @@ from dataclasses import dataclass from datetime import datetime, timezone -from typing import Any, Self +from typing import assert_never +from .._datetime import InvalidDatetime from .._exception import InvalidAttributeError @dataclass(frozen=True, kw_only=True) -class BaseLifetime: - """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. - - 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. - """ - - # 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) -class Lifetime(BaseLifetime): +class Lifetime: """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. + Both timestamps are well-formed [`datetime`][datetime.datetime] values. + A lifetime built from a malformed wire timestamp is an + [`InvalidLifetime`][..InvalidLifetime] instead, so code holding a + `Lifetime` can compare and order its ends without checking them first. + Warning: The [`end_time`][.end_time] timestamp indicates that the asset has been permanently removed from service. @@ -59,8 +35,26 @@ class Lifetime(BaseLifetime): data received from the wire. """ + 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.""" + """Validate this lifetime. + + Raises: + ValueError: If [`start_time`][..start_time] is later than + [`end_time`][..end_time]. + """ if ( self.start_time is not None and self.end_time is not None @@ -97,7 +91,7 @@ def is_operational_now(self) -> bool: @dataclass(frozen=True, kw_only=True) -class InvalidLifetime(BaseLifetime): +class InvalidLifetime: """An operational lifetime with malformed data received from the wire. This class preserves lifetime data that fails the invariants required for @@ -105,17 +99,59 @@ class InvalidLifetime(BaseLifetime): timestamps without accidentally using them for operational checks. Use a semantic accessor, such as `ElectricalComponent.get_operational_lifetime()`, to receive a clear [`InvalidLifetimeError`][..InvalidLifetimeError]. + + Either end may also be an [`InvalidDatetime`][...InvalidDatetime], for a + wire timestamp that is not a well-formed protobuf `Timestamp`. This class + enforces no invariants, so it provides no operational checks and no + accessors: code that reaches an `InvalidLifetime` is already handling + malformed data and reads the two fields directly. + """ + + start_time: datetime | InvalidDatetime | None = None + """The moment when the asset became operationally active. + + `None` when the wire did not set it. An + [`InvalidDatetime`][....InvalidDatetime] when the wire set a malformed + timestamp. + """ + + end_time: datetime | InvalidDatetime | None = None + """The moment when the asset's operational activity ceased. + + `None` when the wire did not set it. An + [`InvalidDatetime`][....InvalidDatetime] when the wire set a malformed + timestamp. """ 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" + start_str = _format_time(self.start_time, unset="-inf") + end_str = _format_time(self.end_time, unset="+inf") return f"" +def _format_time(value: datetime | InvalidDatetime | None, *, unset: str) -> str: + """Render one end of an invalid lifetime range. + + Args: + value: The raw field value. + unset: The text to use when the field is unset. + + Returns: + The ISO 8601 representation of a well-formed timestamp, the invalid + marker of a malformed one, or `unset`. + """ + match value: + case None: + return unset + case datetime() as valid: + return valid.isoformat() + case InvalidDatetime() as invalid: + return str(invalid) + case unknown: + assert_never(unknown) + + class InvalidLifetimeError(InvalidAttributeError): """Raised when a semantic accessor sees an invalid lifetime. diff --git a/src/frequenz/client/common/microgrid/_microgrid.py b/src/frequenz/client/common/microgrid/_microgrid.py index bac17238..f83f3ff2 100644 --- a/src/frequenz/client/common/microgrid/_microgrid.py +++ b/src/frequenz/client/common/microgrid/_microgrid.py @@ -7,6 +7,7 @@ from dataclasses import dataclass, field from typing import assert_never +from .._datetime import InvalidDatetime, InvalidDatetimeError from .._exception import ( MissingFieldError, UnrecognizedEnumValueError, @@ -63,8 +64,17 @@ class Microgrid: # pylint: disable=too-many-instance-attributes location: Location | None """The physical location of the microgrid, in geographical co-ordinates.""" - create_time: datetime.datetime - """The UTC timestamp indicating when the microgrid was initially created.""" + create_time: datetime.datetime | InvalidDatetime + """The UTC timestamp indicating when the microgrid was initially created. + + An [`InvalidDatetime`][....InvalidDatetime] preserves the raw seconds + and nanoseconds when the wire carried a timestamp Python cannot represent. + + Tip: + This is the lower-level field; prefer + [`get_create_time()`][..get_create_time] to obtain a valid + [`datetime`][datetime.datetime] or a clear error. + """ _active: bool | int """Whether the microgrid is active. @@ -180,6 +190,31 @@ def get_delivery_area_or_none(self) -> DeliveryArea | None: case unknown: assert_never(unknown) + def get_create_time(self) -> datetime.datetime: + """Return the creation time as a valid `datetime`. + + This is the higher-level accessor for the [`create_time`][..create_time] + attribute: it resolves the field to a valid + [`datetime`][datetime.datetime] or raises a clear, catchable error. + + Returns: + The creation time, when it is a valid + [`datetime`][datetime.datetime]. + + Raises: + InvalidDatetimeError: If the creation time is an + [`InvalidDatetime`][....InvalidDatetime]. The offending + instance is available on the exception's `datetime` + attribute. + """ + match self.create_time: + case datetime.datetime() as valid: + return valid + case InvalidDatetime() as invalid: + raise InvalidDatetimeError(self, "create_time", invalid) + case unknown: + assert_never(unknown) + def get_location(self) -> Location: """Return the location as a [`Location`][....types.Location]. 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 0ad1baa4..84f06aa1 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 @@ -1265,8 +1265,8 @@ def _get_operational_lifetime_from_proto( Returns: The extracted operational lifetime, an invalid lifetime preserving - malformed timestamp ordering, or an unbounded lifetime if the field - is missing. + malformed timestamp ordering or a timestamp Python cannot + represent, or an unbounded lifetime if the field is missing. """ if message.HasField("operational_lifetime"): return lifetime_from_proto(message.operational_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 a27de45b..0482fda7 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 @@ -62,8 +62,8 @@ def _get_operational_lifetime_from_proto( Returns: The extracted operational lifetime, an invalid lifetime preserving - malformed timestamp ordering, or an unbounded lifetime if the field - is missing. + malformed timestamp ordering or a timestamp Python cannot + represent, or an unbounded lifetime if the field is missing. """ if message.HasField("operational_lifetime"): return lifetime_from_proto(message.operational_lifetime) diff --git a/src/frequenz/client/common/microgrid/proto/v1alpha8/_lifetime.py b/src/frequenz/client/common/microgrid/proto/v1alpha8/_lifetime.py index 9c6451e0..3e3e7fae 100644 --- a/src/frequenz/client/common/microgrid/proto/v1alpha8/_lifetime.py +++ b/src/frequenz/client/common/microgrid/proto/v1alpha8/_lifetime.py @@ -5,7 +5,8 @@ from frequenz.api.common.v1alpha8.microgrid import lifetime_pb2 -from ....proto import datetime_from_proto +from ...._datetime import InvalidDatetime +from ....proto import datetime_from_proto2 from ..._lifetime import InvalidLifetime, Lifetime @@ -16,23 +17,25 @@ def lifetime_from_proto(message: lifetime_pb2.Lifetime) -> Lifetime | InvalidLif message: The protobuf message to convert. Returns: - 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()`. + A [`Lifetime`][....Lifetime] when both timestamps are well-formed and + form a valid range, or an + [`InvalidLifetime`][....InvalidLifetime] preserving a malformed + timestamp or a reversed range. A present but empty protobuf + message becomes an unbounded `Lifetime()`. """ start = ( - datetime_from_proto(message.start_timestamp) + datetime_from_proto2(message.start_timestamp) if message.HasField("start_timestamp") else None ) end = ( - datetime_from_proto(message.end_timestamp) + datetime_from_proto2(message.end_timestamp) if message.HasField("end_timestamp") else None ) - try: - return Lifetime(start_time=start, end_time=end) - except ValueError: - pass + if not isinstance(start, InvalidDatetime) and not isinstance(end, InvalidDatetime): + try: + return Lifetime(start_time=start, end_time=end) + except ValueError: + pass return InvalidLifetime(start_time=start, end_time=end) diff --git a/src/frequenz/client/common/microgrid/proto/v1alpha8/_microgrid.py b/src/frequenz/client/common/microgrid/proto/v1alpha8/_microgrid.py index 5a7ae505..38bd8361 100644 --- a/src/frequenz/client/common/microgrid/proto/v1alpha8/_microgrid.py +++ b/src/frequenz/client/common/microgrid/proto/v1alpha8/_microgrid.py @@ -7,7 +7,7 @@ from ....grid import DeliveryArea, InvalidDeliveryArea from ....grid.proto.v1alpha8 import delivery_area_from_proto2 -from ....proto import datetime_from_proto +from ....proto import datetime_from_proto2 from ....types import Location from ....types.proto.v1alpha8 import location_from_proto from ..._ids import EnterpriseId, MicrogridId @@ -38,6 +38,10 @@ def _microgrid_status_to_active(value: int) -> bool | int: def microgrid_from_proto(message: microgrid_pb2.Microgrid) -> Microgrid: """Convert a protobuf message to a [`Microgrid`][....Microgrid] object. + Malformed input is surfaced through the returned object rather than a side + channel: a malformed delivery area becomes an `InvalidDeliveryArea` and a + creation time Python cannot represent an `InvalidDatetime`. + Args: message: The protobuf message to convert. @@ -59,7 +63,7 @@ def microgrid_from_proto(message: microgrid_pb2.Microgrid) -> Microgrid: name=message.name, delivery_area=delivery_area, location=location, - create_time=datetime_from_proto(message.create_timestamp), + create_time=datetime_from_proto2(message.create_timestamp), _active=_microgrid_status_to_active(message.status), _allow_construction=True, ) diff --git a/src/frequenz/client/common/proto/__init__.py b/src/frequenz/client/common/proto/__init__.py index 0212ad5e..7ce6aa53 100644 --- a/src/frequenz/client/common/proto/__init__.py +++ b/src/frequenz/client/common/proto/__init__.py @@ -3,11 +3,12 @@ """General utilities for converting common types to/from protobuf types.""" +from ._datetime import datetime_from_proto, datetime_from_proto2, datetime_to_proto from ._enum import enum_from_proto -from ._timestamp import datetime_from_proto, datetime_to_proto __all__ = [ "datetime_from_proto", + "datetime_from_proto2", "datetime_to_proto", "enum_from_proto", ] diff --git a/src/frequenz/client/common/proto/_datetime.py b/src/frequenz/client/common/proto/_datetime.py new file mode 100644 index 00000000..80bf1f0c --- /dev/null +++ b/src/frequenz/client/common/proto/_datetime.py @@ -0,0 +1,148 @@ +# License: MIT +# Copyright © 2024 Frequenz Energy-as-a-Service GmbH + +"""Helper functions to convert protobuf Timestamp <-> Python datetime.""" + +from datetime import datetime, timedelta, timezone +from typing import overload + +from google.protobuf import timestamp_pb2 +from typing_extensions import deprecated + +from .._datetime import InvalidDatetime + +_EPOCH = datetime(1970, 1, 1, tzinfo=timezone.utc) +"""The Unix epoch, the instant a protobuf `Timestamp` counts seconds from.""" + +_MAX_NANOS = 999_999_999 +"""The largest fraction of a second a protobuf `Timestamp` may carry.""" + + +@overload +def datetime_to_proto(dt: datetime) -> timestamp_pb2.Timestamp: + """Convert a datetime to a protobuf Timestamp. + + Args: + dt: The datetime object to convert. + + Returns: + The datetime converted to a Timestamp. + """ + + +@overload +def datetime_to_proto(dt: None) -> None: + """Return `None` for a `None` input. + + Args: + dt: None + + Returns: + None + """ + + +def datetime_to_proto(dt: datetime | None) -> timestamp_pb2.Timestamp | None: + """Convert a datetime to a protobuf Timestamp. + + Args: + dt: The datetime object to convert. + + Returns: + The datetime converted to a Timestamp, or `None` if `dt` is `None`. + """ + if dt is None: + return None + + ts = timestamp_pb2.Timestamp() + ts.FromDatetime(dt) + return ts + + +@deprecated( + "`datetime_from_proto` is deprecated; use " + "`datetime_from_proto2` (returns `datetime | InvalidDatetime`) instead." +) +def datetime_from_proto( # noqa: DOC502 + ts: timestamp_pb2.Timestamp, tz: timezone = timezone.utc +) -> datetime: + """Convert a protobuf Timestamp to a datetime. + + Warning: Deprecated + Use [`datetime_from_proto2`][..datetime_from_proto2] instead. The new + conversion function keeps a malformed timestamp in its return type + (`datetime | InvalidDatetime`) rather than raising or silently + repairing it, and is exact across the whole protobuf range, where this + function loses sub-second precision far from the epoch. It always + returns UTC; call [`astimezone()`][datetime.datetime.astimezone] on the + result instead of passing `tz`. + + Args: + ts: The Timestamp object to convert. + tz: The timezone to use for the datetime. + + Returns: + The Timestamp converted to a datetime. + + Raises: + ValueError: If the timestamp has no [`datetime`][datetime.datetime] + equivalent in `tz`. + OverflowError: If the timestamp is so far from the epoch that the + conversion itself overflows. + OSError: If the underlying platform call fails. + """ + # Add microseconds and add nanoseconds converted to microseconds + microseconds = int(ts.nanos / 1000) + return datetime.fromtimestamp(ts.seconds + microseconds * 1e-6, tz=tz) + + +def datetime_from_proto2( + ts: timestamp_pb2.Timestamp, +) -> datetime | InvalidDatetime: + """Convert a protobuf Timestamp to a UTC datetime, preserving invalid data. + + A protobuf `Timestamp` is a `seconds` count since the Unix epoch plus a + `nanos` fraction. Both are plain integers on the wire, so a decoded message + can carry values the `Timestamp` contract does not allow. This function + keeps those in its return type instead of raising, so the caller still + receives what the server sent. + + A timestamp is well-formed when `seconds` is in + `[-62135596800, 253402300799]` — the years 1 to 9999 — and `nanos` is in + `[0, 999999999]`. That `seconds` range is exactly the range + [`datetime`][datetime.datetime] covers, so every well-formed timestamp has + a UTC [`datetime`][datetime.datetime]. The `nanos` fraction is truncated to + the microsecond resolution of [`datetime`][datetime.datetime]. + + An out-of-range `nanos` is not carried over into `seconds`. The `Timestamp` + contract does not allow it, so its intended meaning is unknown, and + guessing one would hand the caller a plausible-looking timestamp built from + data the sender never promised. + + Note: + The result is always in UTC, which is what a `Timestamp` denotes. Call + [`astimezone()`][datetime.datetime.astimezone] on it for another zone. + That conversion can itself overflow within a day of either end of the + range, which is why it is left to the caller rather than hidden in a + `tz` argument here. + + Args: + ts: The Timestamp object to convert. + + Returns: + The Timestamp converted to a UTC datetime, or an + [`InvalidDatetime`][frequenz.client.common.InvalidDatetime] + carrying the raw `seconds` and `nanos` when the timestamp is not + well-formed. + """ + seconds = ts.seconds + nanos = ts.nanos + if not 0 <= nanos <= _MAX_NANOS: + return InvalidDatetime(seconds=seconds, nanos=nanos) + try: + # Going through `timedelta` instead of `datetime.fromtimestamp()` keeps + # the conversion exact: a `float` timestamp cannot hold microsecond + # resolution across the whole protobuf range. + return _EPOCH + timedelta(seconds=seconds, microseconds=nanos // 1000) + except OverflowError: + return InvalidDatetime(seconds=seconds, nanos=nanos) diff --git a/src/frequenz/client/common/proto/_timestamp.py b/src/frequenz/client/common/proto/_timestamp.py deleted file mode 100644 index c7fe8962..00000000 --- a/src/frequenz/client/common/proto/_timestamp.py +++ /dev/null @@ -1,67 +0,0 @@ -# License: MIT -# Copyright © 2024 Frequenz Energy-as-a-Service GmbH - -"""Helper functions to convert protobuf Timestamp <-> Python datetime.""" - -from datetime import datetime, timezone -from typing import overload - -from google.protobuf import timestamp_pb2 - - -@overload -def datetime_to_proto(dt: datetime) -> timestamp_pb2.Timestamp: - """Convert a datetime to a protobuf Timestamp. - - Args: - dt: The datetime object to convert. - - Returns: - The datetime converted to a Timestamp. - """ - - -@overload -def datetime_to_proto(dt: None) -> None: - """Return `None` for a `None` input. - - Args: - dt: None - - Returns: - None - """ - - -def datetime_to_proto(dt: datetime | None) -> timestamp_pb2.Timestamp | None: - """Convert a datetime to a protobuf Timestamp. - - Args: - dt: The datetime object to convert. - - Returns: - The datetime converted to a Timestamp, or `None` if `dt` is `None`. - """ - if dt is None: - return None - - ts = timestamp_pb2.Timestamp() - ts.FromDatetime(dt) - return ts - - -def datetime_from_proto( - ts: timestamp_pb2.Timestamp, tz: timezone = timezone.utc -) -> datetime: - """Convert a protobuf Timestamp to a datetime. - - Args: - ts: The Timestamp object to convert. - tz: The timezone to use for the datetime. - - Returns: - The Timestamp converted to a datetime. - """ - # Add microseconds and add nanoseconds converted to microseconds - microseconds = int(ts.nanos / 1000) - return datetime.fromtimestamp(ts.seconds + microseconds * 1e-6, tz=tz) diff --git a/tests/metrics/proto/v1alpha8/test_sample_metric_sample.py b/tests/metrics/proto/v1alpha8/test_sample_metric_sample.py index 4b13bbff..13f85d37 100644 --- a/tests/metrics/proto/v1alpha8/test_sample_metric_sample.py +++ b/tests/metrics/proto/v1alpha8/test_sample_metric_sample.py @@ -13,6 +13,7 @@ from frequenz.api.common.v1alpha8.metrics import bounds_pb2, metrics_pb2 from google.protobuf.timestamp_pb2 import Timestamp +from frequenz.client.common import InvalidDatetime, InvalidDatetimeError from frequenz.client.common.metrics import ( AggregatedMetricValue, Bounds, @@ -378,3 +379,59 @@ def test_from_proto_unspecified_metric() -> None: assert sample.metric == 0 assert not isinstance(sample.metric, Metric) + + +def _sample_with_time(sample_time: Timestamp) -> metrics_pb2.MetricSample: + """Build a minimal well-formed sample carrying the given time. + + Args: + sample_time: The timestamp to put in the `sample_time` field. + + Returns: + The protobuf message. + """ + return metrics_pb2.MetricSample( + sample_time=sample_time, + metric=metric_to_proto(Metric.AC_POWER_ACTIVE), + value=metrics_pb2.MetricValueVariant( + simple_metric=metrics_pb2.SimpleMetricValue(value=5.0) + ), + ) + + +@pytest.mark.parametrize( + "sample_time", + [ + pytest.param(Timestamp(seconds=253402300800), id="year-10000"), + pytest.param(Timestamp(seconds=-62135596801), id="before-year-1"), + pytest.param(Timestamp(seconds=0, nanos=-1), id="negative-nanos"), + pytest.param(Timestamp(seconds=0, nanos=1000000000), id="a-whole-second"), + ], +) +def test_from_proto_unrepresentable_sample_time(sample_time: Timestamp) -> None: + """An unrepresentable sample time is preserved instead of raising.""" + sample = metric_sample_from_proto(_sample_with_time(sample_time)) + + assert sample.sample_time2 == InvalidDatetime( + seconds=sample_time.seconds, nanos=sample_time.nanos + ) + with pytest.raises(InvalidDatetimeError): + sample.get_sample_time() + + +def test_from_proto_with_issues_malformed_sample_time_raises() -> None: + """The released converter keeps raising, as it did before the union.""" + major_issues: list[str] = [] + minor_issues: list[str] = [] + + with ( + pytest.deprecated_call(match="metric_sample_from_proto"), + pytest.raises( + ValueError, match=r"malformed sample_time " + ), + ): + metric_sample_from_proto_with_issues( + _sample_with_time(Timestamp(seconds=253402300800)), + major_issues=major_issues, + minor_issues=minor_issues, + ) diff --git a/tests/metrics/test_sample_metric_sample.py b/tests/metrics/test_sample_metric_sample.py index aeee0140..1b90f58c 100644 --- a/tests/metrics/test_sample_metric_sample.py +++ b/tests/metrics/test_sample_metric_sample.py @@ -9,6 +9,8 @@ from frequenz.core.typing import FloatInt from frequenz.client.common import ( + InvalidDatetime, + InvalidDatetimeError, UnrecognizedEnumValueError, UnspecifiedEnumValueError, ) @@ -72,7 +74,7 @@ def test_creation( bounds_set=bounds_set, connection=connection, ) - assert sample.sample_time == now + assert sample.sample_time2 == now assert sample.metric == Metric.AC_POWER_ACTIVE assert sample.value == value assert sample.bounds_set == bounds_set @@ -372,3 +374,83 @@ def test_get_bounds_set_invalid_raises(now: datetime) -> None: with pytest.raises(InvalidBoundsSetError) as exc_info: sample.get_bounds_set() assert exc_info.value.bounds_set is invalid + + +def test_get_sample_time_returns_valid(now: datetime) -> None: + """get_sample_time returns the time when it is a valid datetime.""" + sample = MetricSample( + sample_time=now, + metric=Metric.AC_POWER_ACTIVE, + value=5.0, + bounds_set=BoundsSet(), + ) + assert sample.get_sample_time() is now + + +def test_get_sample_time_invalid_raises() -> None: + """get_sample_time raises InvalidDatetimeError for an InvalidDatetime.""" + invalid = InvalidDatetime(seconds=253402300800, nanos=0) + sample = MetricSample( + sample_time2=invalid, + metric=Metric.AC_POWER_ACTIVE, + value=5.0, + bounds_set=BoundsSet(), + ) + assert sample.sample_time2 is invalid + with pytest.raises(InvalidDatetimeError) as exc_info: + sample.get_sample_time() + assert exc_info.value.datetime is invalid + assert exc_info.value.attr_name == "sample_time2" + + +def test_deprecated_sample_time_property(now: datetime) -> None: + """The deprecated `sample_time` property still returns the valid datetime.""" + sample = MetricSample( + sample_time=now, + metric=Metric.AC_POWER_ACTIVE, + value=5.0, + bounds_set=BoundsSet(), + ) + assert sample.sample_time2 is now + with pytest.deprecated_call( + match="`MetricSample.sample_time` is deprecated; use `sample_time2` instead." + ): + assert sample.sample_time is now + + +def test_deprecated_sample_time_property_raises_for_invalid() -> None: + """The deprecated property cannot express an `InvalidDatetime`, so it raises.""" + invalid = InvalidDatetime(seconds=253402300800, nanos=0) + sample = MetricSample( + sample_time2=invalid, + metric=Metric.AC_POWER_ACTIVE, + value=5.0, + bounds_set=BoundsSet(), + ) + with pytest.deprecated_call(), pytest.raises(InvalidDatetimeError): + _ = sample.sample_time + + +def test_sample_time_and_sample_time2_raises(now: datetime) -> None: + """Passing both spellings of the sample time is an error.""" + with pytest.raises( + TypeError, + match=r"accepts either `sample_time` or `sample_time2`, not both", + ): + MetricSample( + sample_time=now, + sample_time2=now, + metric=Metric.AC_POWER_ACTIVE, + value=5.0, + bounds_set=BoundsSet(), + ) + + +def test_missing_sample_time_raises() -> None: + """Passing neither spelling of the sample time is an error.""" + with pytest.raises(TypeError, match=r"requires the `sample_time2` argument"): + MetricSample( # pylint: disable=missing-kwoa + metric=Metric.AC_POWER_ACTIVE, + value=5.0, + bounds_set=BoundsSet(), + ) diff --git a/tests/microgrid/_lifetime/test_base_lifetime.py b/tests/microgrid/_lifetime/test_base_lifetime.py deleted file mode 100644 index 7b633c86..00000000 --- a/tests/microgrid/_lifetime/test_base_lifetime.py +++ /dev/null @@ -1,14 +0,0 @@ -# License: MIT -# Copyright © 2026 Frequenz Energy-as-a-Service GmbH - -"""Tests for `BaseLifetime`.""" - -import pytest - -from frequenz.client.common.microgrid 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/microgrid/_lifetime/test_invalid_lifetime.py b/tests/microgrid/_lifetime/test_invalid_lifetime.py index 1b3ffc3e..24e83b7f 100644 --- a/tests/microgrid/_lifetime/test_invalid_lifetime.py +++ b/tests/microgrid/_lifetime/test_invalid_lifetime.py @@ -8,7 +8,8 @@ import pytest -from frequenz.client.common.microgrid import BaseLifetime, InvalidLifetime +from frequenz.client.common import InvalidDatetime +from frequenz.client.common.microgrid import InvalidLifetime @dataclass(frozen=True, kw_only=True) @@ -18,21 +19,16 @@ class _StrTestCase: name: str """The description of the test case.""" - start_time: datetime | None + start_time: datetime | InvalidDatetime | None """The start time to use for the invalid lifetime.""" - end_time: datetime | None + end_time: datetime | InvalidDatetime | 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) - - 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) @@ -41,6 +37,15 @@ def test_accepts_invalid_range(present: datetime, future: datetime) -> None: assert lifetime.end_time is present +def test_accepts_invalid_datetime(present: datetime) -> None: + """`InvalidLifetime` preserves a timestamp with no `datetime` equivalent.""" + invalid = InvalidDatetime(seconds=253402300800, nanos=0) + lifetime = InvalidLifetime(start_time=present, end_time=invalid) + + assert lifetime.start_time is present + assert lifetime.end_time is invalid + + @pytest.mark.parametrize( "case", [ @@ -70,6 +75,14 @@ def test_accepts_invalid_range(present: datetime, future: datetime) -> None: end_time=None, expected_str="", ), + _StrTestCase( + name="unrepresentable_start", + start_time=InvalidDatetime(seconds=253402300800, nanos=0), + end_time=datetime(2025, 1, 1, 12, 0, 0, tzinfo=timezone.utc), + expected_str=( + ",2025-01-01T12:00:00+00:00]>" + ), + ), ], ids=lambda case: case.name, ) diff --git a/tests/microgrid/_lifetime/test_lifetime.py b/tests/microgrid/_lifetime/test_lifetime.py index f6757570..0587f2ff 100644 --- a/tests/microgrid/_lifetime/test_lifetime.py +++ b/tests/microgrid/_lifetime/test_lifetime.py @@ -9,7 +9,7 @@ import pytest -from frequenz.client.common.microgrid import BaseLifetime, Lifetime +from frequenz.client.common.microgrid import Lifetime class _Time(Enum): @@ -96,11 +96,6 @@ class _StrTestCase: """The expected string representation.""" -def test_is_base_lifetime_subclass() -> None: - """`Lifetime` is a subclass of `BaseLifetime`.""" - assert issubclass(Lifetime, BaseLifetime) - - @pytest.mark.parametrize( "case", [ diff --git a/tests/microgrid/electrical_components/proto/v1alpha8/conftest.py b/tests/microgrid/electrical_components/proto/v1alpha8/conftest.py index 4f2fdfd6..3e445151 100644 --- a/tests/microgrid/electrical_components/proto/v1alpha8/conftest.py +++ b/tests/microgrid/electrical_components/proto/v1alpha8/conftest.py @@ -142,7 +142,9 @@ def base_data_as_proto( (provides_telemetry, accepts_control) ], ) - if base_data.lifetime: + # This builder only ever receives valid lifetimes; a malformed timestamp has + # no protobuf spelling to write back. + if isinstance(base_data.lifetime, Lifetime): lifetime_dict: dict[str, Timestamp] = {} if base_data.lifetime.start_time is not None: lifetime_dict["start_timestamp"] = datetime_to_proto( diff --git a/tests/microgrid/proto/v1alpha8/test_lifetime.py b/tests/microgrid/proto/v1alpha8/test_lifetime.py index c901b657..9cbbff86 100644 --- a/tests/microgrid/proto/v1alpha8/test_lifetime.py +++ b/tests/microgrid/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 import InvalidDatetime from frequenz.client.common.microgrid import InvalidLifetime from frequenz.client.common.microgrid.proto.v1alpha8 import lifetime_from_proto @@ -115,3 +116,37 @@ def test_from_proto_preserves_start_after_end( assert isinstance(lifetime, InvalidLifetime) assert lifetime.start_time == future assert lifetime.end_time == now + + +@pytest.mark.parametrize("field_name", ["start_timestamp", "end_timestamp"]) +@pytest.mark.parametrize( + "unrepresentable", + [ + pytest.param(timestamp_pb2.Timestamp(seconds=253402300800), id="year-10000"), + pytest.param( + timestamp_pb2.Timestamp(seconds=0, nanos=1000000000), + id="a-whole-second-of-nanos", + ), + pytest.param(timestamp_pb2.Timestamp(seconds=0, nanos=-1), id="negative-nanos"), + ], +) +def test_from_proto_preserves_unrepresentable_timestamp( + now: datetime, field_name: str, unrepresentable: timestamp_pb2.Timestamp +) -> None: + """A timestamp with no `datetime` equivalent makes the whole lifetime invalid.""" + now_ts = timestamp_pb2.Timestamp() + now_ts.FromDatetime(now) + + proto_kwargs: dict[str, Any] = { + "start_timestamp": now_ts, + "end_timestamp": now_ts, + } + proto_kwargs[field_name] = unrepresentable + + lifetime = lifetime_from_proto(lifetime_pb2.Lifetime(**proto_kwargs)) + attr_name = field_name.replace("timestamp", "time") + + assert isinstance(lifetime, InvalidLifetime) + assert getattr(lifetime, attr_name) == InvalidDatetime( + seconds=unrepresentable.seconds, nanos=unrepresentable.nanos + ) diff --git a/tests/microgrid/proto/v1alpha8/test_microgrid.py b/tests/microgrid/proto/v1alpha8/test_microgrid.py index 2297df3e..67a92435 100644 --- a/tests/microgrid/proto/v1alpha8/test_microgrid.py +++ b/tests/microgrid/proto/v1alpha8/test_microgrid.py @@ -11,7 +11,12 @@ from frequenz.api.common.v1alpha8.grid import delivery_area_pb2 from frequenz.api.common.v1alpha8.microgrid import microgrid_pb2 +# pylint: disable-next=no-name-in-module +from google.protobuf.timestamp_pb2 import Timestamp + from frequenz.client.common import ( + InvalidDatetime, + InvalidDatetimeError, UnrecognizedEnumValueError, UnspecifiedEnumValueError, ) @@ -131,16 +136,18 @@ def _assert_active(info: Microgrid, expected_active: bool | int) -> None: "frequenz.client.common.microgrid.proto.v1alpha8._microgrid.delivery_area_from_proto2" ) @patch("frequenz.client.common.microgrid.proto.v1alpha8._microgrid.location_from_proto") -@patch("frequenz.client.common.microgrid.proto.v1alpha8._microgrid.datetime_from_proto") +@patch( + "frequenz.client.common.microgrid.proto.v1alpha8._microgrid.datetime_from_proto2" +) def test_from_proto( - mock_datetime_from_proto: Mock, + mock_datetime_from_proto2: Mock, mock_location_from_proto: Mock, mock_delivery_area_from_proto: Mock, case: _ProtoConversionTestCase, ) -> None: """Test conversion from protobuf message to Microgrid.""" now = datetime.now(timezone.utc) - mock_datetime_from_proto.return_value = now + mock_datetime_from_proto2.return_value = now mock_location = ( Location( @@ -198,7 +205,7 @@ def test_from_proto( _assert_active(info, case.expected_active) # Verify mock calls - mock_datetime_from_proto.assert_called_once_with(proto.create_timestamp) + mock_datetime_from_proto2.assert_called_once_with(proto.create_timestamp) if case.has_delivery_area: mock_delivery_area_from_proto.assert_called_once_with(proto.delivery_area) @@ -213,3 +220,30 @@ def test_from_proto( else: mock_location_from_proto.assert_not_called() assert info.location is None + + +@pytest.mark.parametrize( + "create_timestamp", + [ + pytest.param(Timestamp(seconds=253402300800), id="year-10000"), + pytest.param(Timestamp(seconds=0, nanos=1000000000), id="a-whole-second"), + pytest.param(Timestamp(seconds=0, nanos=-1), id="negative-nanos"), + ], +) +def test_from_proto_unrepresentable_create_time(create_timestamp: Timestamp) -> None: + """An unrepresentable creation time is preserved instead of raising.""" + proto = microgrid_pb2.Microgrid( + id=1234, + enterprise_id=5678, + name="Test Grid", + status=microgrid_pb2.MICROGRID_STATUS_ACTIVE, + create_timestamp=create_timestamp, + ) + + info = microgrid_from_proto(proto) + + assert info.create_time == InvalidDatetime( + seconds=create_timestamp.seconds, nanos=create_timestamp.nanos + ) + with pytest.raises(InvalidDatetimeError): + info.get_create_time() diff --git a/tests/microgrid/test_microgrid.py b/tests/microgrid/test_microgrid.py index 7a56bbd0..fa014fc4 100644 --- a/tests/microgrid/test_microgrid.py +++ b/tests/microgrid/test_microgrid.py @@ -9,6 +9,8 @@ import pytest from frequenz.client.common import ( + InvalidDatetime, + InvalidDatetimeError, MissingFieldError, UnrecognizedEnumValueError, UnspecifiedEnumValueError, @@ -198,6 +200,7 @@ def test_replace_preserves_construction() -> None: def _make_microgrid( delivery_area: DeliveryArea | InvalidDeliveryArea | None = None, location: Location | None = None, + create_time: datetime | InvalidDatetime | None = None, ) -> Microgrid: """Build a Microgrid with the given delivery area and location for accessor tests.""" return Microgrid( @@ -206,7 +209,9 @@ def _make_microgrid( name="", delivery_area=delivery_area, location=location, - create_time=datetime.now(timezone.utc), + create_time=( + datetime.now(timezone.utc) if create_time is None else create_time + ), _active=True, _allow_construction=True, ) @@ -311,3 +316,31 @@ def test_get_location_error_is_value_error() -> None: info = _make_microgrid() with pytest.raises(ValueError): info.get_location() + + +def test_get_create_time_returns_valid() -> None: + """`get_create_time()` returns the stored `datetime` unchanged.""" + now = datetime.now(timezone.utc) + info = _make_microgrid(create_time=now) + assert info.get_create_time() is now + + +def test_get_create_time_raises_invalid_for_invalid_datetime() -> None: + """`get_create_time()` raises `InvalidDatetimeError` for an `InvalidDatetime`.""" + invalid = InvalidDatetime(seconds=253402300800, nanos=0) + info = _make_microgrid(create_time=invalid) + assert info.create_time is invalid + with pytest.raises( + InvalidDatetimeError, + match=r"invalid timestamp for attribute " + r"'create_time' in MID1234", + ) as exc_info: + info.get_create_time() + assert exc_info.value.datetime is invalid + + +def test_get_create_time_error_is_value_error() -> None: + """The `InvalidDatetimeError` raised by the accessor is also a `ValueError`.""" + info = _make_microgrid(create_time=InvalidDatetime(seconds=0, nanos=-1)) + with pytest.raises(ValueError): + info.get_create_time() diff --git a/tests/proto/test_datetime.py b/tests/proto/test_datetime.py index 82c6eb49..eb2b82d8 100644 --- a/tests/proto/test_datetime.py +++ b/tests/proto/test_datetime.py @@ -3,7 +3,9 @@ """Test conversion helper functions.""" -from datetime import datetime, timezone +from datetime import datetime, timedelta, timezone + +import pytest # pylint: disable=no-name-in-module from google.protobuf.timestamp_pb2 import Timestamp @@ -12,7 +14,17 @@ from hypothesis import given from hypothesis import strategies as st -from frequenz.client.common.proto import datetime_from_proto, datetime_to_proto +from frequenz.client.common import InvalidDatetime +from frequenz.client.common.proto import ( + datetime_from_proto, + datetime_from_proto2, + datetime_to_proto, +) + +# The oldest and newest instants both protobuf and Python can represent. +_MIN_SECONDS = -62135596800 # 0001-01-01T00:00:00Z +_MAX_SECONDS = 253402300799 # 9999-12-31T23:59:59Z +_MAX_NANOS = 999999999 # Strategy for generating datetime objects # It requires naive datetime objects because it creates the timezone via a strategy @@ -37,9 +49,7 @@ def test_to_timestamp_with_datetime(dt: datetime) -> None: """Test conversion from datetime to Timestamp.""" ts = datetime_to_proto(dt) assert ts is not None - converted_back_dt = datetime_from_proto(ts) - assert dt.tzinfo == converted_back_dt.tzinfo - assert dt.timestamp() == converted_back_dt.timestamp() + assert datetime_from_proto2(ts) == dt def test_to_timestamp_with_none() -> None: @@ -50,8 +60,8 @@ def test_to_timestamp_with_none() -> None: @given(timestamp_strategy) def test_to_datetime(ts: Timestamp) -> None: """Test conversion from Timestamp to datetime.""" - dt = datetime_from_proto(ts) - assert dt is not None + dt = datetime_from_proto2(ts) + assert isinstance(dt, datetime) # Convert back to Timestamp and compare converted_back_ts = datetime_to_proto(dt) assert ts.seconds == converted_back_ts.seconds @@ -68,3 +78,127 @@ def test_no_none_datetime(dt: datetime) -> None: assert ts is not None assert ts2 is None + + +def test_from_proto_is_deprecated() -> None: + """`datetime_from_proto` warns and still converts as it always did.""" + with pytest.deprecated_call( + match=r"`datetime_from_proto` is deprecated; use `datetime_from_proto2` " + r"\(returns `datetime \| InvalidDatetime`\) instead\." + ): + converted = datetime_from_proto(Timestamp(seconds=1, nanos=500000000)) + assert converted == datetime(1970, 1, 1, 0, 0, 1, 500000, tzinfo=timezone.utc) + + +def test_from_proto_still_raises_out_of_range() -> None: + """`datetime_from_proto` keeps raising, which is why it is deprecated.""" + with pytest.deprecated_call(), pytest.raises((ValueError, OverflowError)): + datetime_from_proto(Timestamp(seconds=_MAX_SECONDS + 1)) + + +def test_from_proto2_epoch() -> None: + """An all-zero timestamp is the Unix epoch in UTC.""" + assert datetime_from_proto2(Timestamp()) == datetime( + 1970, 1, 1, tzinfo=timezone.utc + ) + + +def test_from_proto2_truncates_nanos_to_microseconds() -> None: + """Sub-microsecond precision is truncated, not rounded.""" + assert datetime_from_proto2(Timestamp(seconds=0, nanos=1999)) == datetime( + 1970, 1, 1, 0, 0, 0, 1, tzinfo=timezone.utc + ) + + +def test_from_proto2_negative_seconds() -> None: + """A `nanos` fraction is added to (not subtracted from) negative seconds.""" + assert datetime_from_proto2(Timestamp(seconds=-1, nanos=500000000)) == datetime( + 1969, 12, 31, 23, 59, 59, 500000, tzinfo=timezone.utc + ) + + +def test_from_proto2_min() -> None: + """The oldest instant protobuf allows is representable.""" + assert datetime_from_proto2(Timestamp(seconds=_MIN_SECONDS)) == datetime( + 1, 1, 1, tzinfo=timezone.utc + ) + + +def test_from_proto2_max() -> None: + """The newest instant protobuf allows is representable, down to the microsecond.""" + assert datetime_from_proto2( + Timestamp(seconds=_MAX_SECONDS, nanos=_MAX_NANOS) + ) == datetime(9999, 12, 31, 23, 59, 59, 999999, tzinfo=timezone.utc) + + +def test_from_proto2_is_always_utc() -> None: + """The result is aware and in UTC, which is what a `Timestamp` denotes.""" + converted = datetime_from_proto2(Timestamp(seconds=0)) + assert isinstance(converted, datetime) + assert converted.tzinfo is timezone.utc + + +def test_from_proto2_result_converts_to_another_zone() -> None: + """The caller expresses the instant in another zone themselves.""" + tz = timezone(timedelta(hours=5, minutes=30)) + converted = datetime_from_proto2(Timestamp(seconds=0)) + assert isinstance(converted, datetime) + in_tz = converted.astimezone(tz) + assert in_tz == converted + assert (in_tz.hour, in_tz.minute) == (5, 30) + + +@pytest.mark.parametrize( + "seconds", + [ + pytest.param(_MAX_SECONDS + 1, id="above-max"), + pytest.param(_MIN_SECONDS - 1, id="below-min"), + pytest.param(2**63 - 1, id="int64-max"), + pytest.param(-(2**63), id="int64-min"), + ], +) +def test_from_proto2_seconds_out_of_python_range(seconds: int) -> None: + """A seconds count outside the years 1 to 9999 is preserved, not raised.""" + assert datetime_from_proto2(Timestamp(seconds=seconds)) == InvalidDatetime( + seconds=seconds, nanos=0 + ) + + +@pytest.mark.parametrize( + "nanos", + [ + pytest.param(-1, id="negative"), + pytest.param(_MAX_NANOS + 1, id="a-whole-second"), + pytest.param(-(2**31), id="int32-min"), + pytest.param(2**31 - 1, id="int32-max"), + ], +) +def test_from_proto2_nanos_out_of_spec(nanos: int) -> None: + """A `nanos` fraction outside `[0, 999999999]` is preserved, not normalized.""" + assert datetime_from_proto2(Timestamp(seconds=1, nanos=nanos)) == InvalidDatetime( + seconds=1, nanos=nanos + ) + + +def test_from_proto2_out_of_range_survives_the_wire() -> None: + """A value only a decoded message can carry round-trips into the wrapper.""" + raw = Timestamp(seconds=_MAX_SECONDS + 1, nanos=-1).SerializeToString() + decoded = Timestamp() + decoded.ParseFromString(raw) + assert datetime_from_proto2(decoded) == InvalidDatetime( + seconds=_MAX_SECONDS + 1, nanos=-1 + ) + + +def test_from_proto2_accepts_every_well_formed_timestamp() -> None: + """The protobuf `seconds` range is exactly the range `datetime` covers. + + That equality is what lets a single `InvalidDatetime` mean "not a + well-formed `Timestamp`" without also having to mean "valid but not + representable". + """ + for seconds in (_MIN_SECONDS, 0, _MAX_SECONDS): + for nanos in (0, _MAX_NANOS): + assert isinstance( + datetime_from_proto2(Timestamp(seconds=seconds, nanos=nanos)), datetime + ) diff --git a/tests/test_datetime.py b/tests/test_datetime.py new file mode 100644 index 00000000..6967516f --- /dev/null +++ b/tests/test_datetime.py @@ -0,0 +1,73 @@ +# License: MIT +# Copyright © 2026 Frequenz Energy-as-a-Service GmbH + +"""Tests for the `InvalidDatetime` wrapper type and its error.""" + +from frequenz.client.common import ( + ClientCommonError, + InvalidAttributeError, + InvalidDatetime, + InvalidDatetimeError, +) + + +def test_stores_raw_numbers() -> None: + """`InvalidDatetime` stores the raw wire numbers verbatim.""" + invalid = InvalidDatetime(seconds=253402300800, nanos=1) + assert invalid.seconds == 253402300800 + assert invalid.nanos == 1 + + +def test_equality() -> None: + """Two `InvalidDatetime` with the same numbers are equal and hash the same.""" + a = InvalidDatetime(seconds=253402300800, nanos=1) + b = InvalidDatetime(seconds=253402300800, nanos=1) + assert a == b + assert hash(a) == hash(b) + assert a != InvalidDatetime(seconds=253402300800, nanos=2) + + +def test_str() -> None: + """`InvalidDatetime.__str__` renders with a compact invalid marker.""" + assert str(InvalidDatetime(seconds=253402300800, nanos=0)) == ( + "" + ) + + +def test_str_negative_nanos() -> None: + """A negative fraction keeps its sign, so the two numbers stay readable.""" + assert str(InvalidDatetime(seconds=-1, nanos=-1)) == "" + + +def test_error_inherits_invalid_attribute_error() -> None: + """`InvalidDatetimeError` inherits `InvalidAttributeError` (and thus `ValueError`).""" + assert issubclass(InvalidDatetimeError, InvalidAttributeError) + assert issubclass(InvalidDatetimeError, ClientCommonError) + assert issubclass(InvalidDatetimeError, ValueError) + + +def test_error_stores_instance_attr_name_and_datetime() -> None: + """`InvalidDatetimeError` stores `instance`, `attr_name` and `datetime`.""" + instance = object() + invalid = InvalidDatetime(seconds=253402300800, nanos=0) + error = InvalidDatetimeError(instance, "sample_time", invalid) + assert error.instance is instance + assert error.attr_name == "sample_time" + assert error.datetime is invalid + + +def test_error_default_message() -> None: + """The default message follows the `invalid timestamp ...` template.""" + invalid = InvalidDatetime(seconds=253402300800, nanos=0) + assert str(InvalidDatetimeError("some-instance", "sample_time", invalid)) == ( + "invalid timestamp for attribute " + "'sample_time' in some-instance" + ) + + +def test_error_custom_message_replaces_the_default() -> None: + """A custom message replaces the default entirely.""" + invalid = InvalidDatetime(seconds=0, nanos=-1) + assert str(InvalidDatetimeError("i", "a", invalid, "explicit msg")) == ( + "explicit msg" + )