Validate start/end times in the reporting client and CLI

README.md

@@ -82,8 +82,8 @@ data = [
             Metric.AC_ACTIVE_POWER,      # AC active power
             Metric.AC_REACTIVE_POWER,      # AC reactive power
         ],
-        start_time=datetime.fromisoformat("2024-05-01T00:00:00"),  # Start of query range (UTC)
-        end_time=datetime.fromisoformat("2024-05-02T00:00:00"),    # End of query range (UTC)
+        start_time=datetime.fromisoformat("2024-05-01T00:00:00+00:00"),  # Start of query range (UTC)
+        end_time=datetime.fromisoformat("2024-05-02T00:00:00+00:00"),    # End of query range (UTC)
         resampling_period=timedelta(seconds=5),  # Optional: downsample data to 5-second intervals
     )
 ]
@@ -101,8 +101,8 @@ data = [
         microgrid_id=1,
         sensor_id=100,
         metrics=[Metric.SENSOR_IRRADIANCE],
-        start_time=datetime.fromisoformat("2024-05-01T00:00:00"),
-        end_time=datetime.fromisoformat("2024-05-02T00:00:00"),
+        start_time=datetime.fromisoformat("2024-05-01T00:00:00+00:00"),
+        end_time=datetime.fromisoformat("2024-05-02T00:00:00+00:00"),
         resampling_period=timedelta(seconds=1),
     )
 ]
@@ -130,8 +130,8 @@ data = [
     client.receive_microgrid_components_data(
         microgrid_components=microgrid_components,
         metrics=[Metric.AC_ACTIVE_POWER, Metric.AC_REACTIVE_POWER],
-        start_time=datetime.fromisoformat("2024-05-01T00:00:00"),
-        end_time=datetime.fromisoformat("2024-05-02T00:00:00"),
+        start_time=datetime.fromisoformat("2024-05-01T00:00:00+00:00"),
+        end_time=datetime.fromisoformat("2024-05-02T00:00:00+00:00"),
         resampling_period=timedelta(seconds=1),
         include_states=False, # Set to True to include state data
         include_bounds=False, # Set to True to include metric bounds data
@@ -160,8 +160,8 @@ data = [
     client.receive_microgrid_sensors_data(
         microgrid_sensors=microgrid_sensors,
         metrics=[Metric.SENSOR_IRRADIANCE],
-        start_time=datetime.fromisoformat("2024-05-01T00:00:00"),
-        end_time=datetime.fromisoformat("2024-05-02T00:00:00"),
+        start_time=datetime.fromisoformat("2024-05-01T00:00:00+00:00"),
+        end_time=datetime.fromisoformat("2024-05-02T00:00:00+00:00"),
         resampling_period=timedelta(seconds=1),
         include_states=False, # Set to True to include state data
     )
@@ -183,8 +183,8 @@ data = [
         microgrid_id=microgrid_id,
         metric=Metric.AC_ACTIVE_POWER,
         aggregation_formula=formula,
-        start_time=datetime.fromisoformat("2024-05-01T00:00:00"),
-        end_time=datetime.fromisoformat("2024-05-02T00:00:00"),
+        start_time=datetime.fromisoformat("2024-05-01T00:00:00+00:00"),
+        end_time=datetime.fromisoformat("2024-05-02T00:00:00+00:00"),
         resampling_period=resampling_period,
     )
 ]

RELEASE_NOTES.md

@@ -6,11 +6,11 @@
 
 ## Upgrading
 
-<!-- Here goes notes on how to upgrade from previous versions, including deprecations and what they should be replaced with -->
+- `start_time` and `end_time` must now be timezone-aware. The client raises `ValueError` on a naive datetime instead of letting it be read inconsistently (UTC on the wire, local time elsewhere), and the CLI `--start`/`--end` reject a value without an offset. Add an offset such as `+00:00` to existing naive values.
 
 ## New Features
 
-<!-- Here goes the main new features and examples or instructions on how to use them -->
+- The CLI now warns when `--end` is before `--start`. The request still runs, but the inverted range returns no data, so the warning flags what is almost always a typo.
 
 ## Bug Fixes
 

src/frequenz/client/reporting/_client.py

@@ -63,6 +63,24 @@
 )
 
 
+def _reject_naive(**times: datetime | None) -> None:
+    """Raise if any given datetime is timezone-naive.
+
+    A naive datetime is ambiguous: the wire encoding assumes UTC while
+    `datetime.timestamp()` assumes local time, so the same value would denote
+    different instants.
+
+    Args:
+        **times: datetimes to check, keyed by parameter name for the error message.
+
+    Raises:
+        ValueError: if any value is a naive datetime.
+    """
+    for name, dt in times.items():
+        if dt is not None and dt.tzinfo is None:
+            raise ValueError(f"{name} must be timezone-aware, got naive {dt!r}")
+
+
 class ReportingApiClient(BaseApiClient[ReportingStub]):
     """A client for the Reporting service."""
 
@@ -145,7 +163,7 @@ def stub(self) -> ReportingStub:
         return self._stub
 
     # pylint: disable=too-many-arguments
-    def receive_single_component_data(
+    def receive_single_component_data(  # noqa: DOC502
         self,
         *,
         microgrid_id: int,
@@ -171,6 +189,9 @@ def receive_single_component_data(
 
         Returns:
             A receiver of `MetricSample`s.
+
+        Raises:
+            ValueError: If start_time or end_time is timezone-naive.
         """
         receiver = self._receive_microgrid_components_data_batch(
             microgrid_components=[(microgrid_id, [component_id])],
@@ -185,7 +206,7 @@ def receive_single_component_data(
         return BatchUnrollReceiver(receiver)
 
     # pylint: disable=too-many-arguments
-    def receive_microgrid_components_data(
+    def receive_microgrid_components_data(  # noqa: DOC502
         self,
         *,
         microgrid_components: list[tuple[int, list[int]]],
@@ -210,6 +231,9 @@ def receive_microgrid_components_data(
 
         Returns:
             A receiver of `MetricSample`s.
+
+        Raises:
+            ValueError: If start_time or end_time is timezone-naive.
         """
         receiver = self._receive_microgrid_components_data_batch(
             microgrid_components=microgrid_components,
@@ -237,6 +261,7 @@ def _receive_microgrid_components_data_batch(
         include_bounds: bool = False,
     ) -> Receiver[ComponentsDataBatch]:
         """Return a Receiver for the microgrid component data stream."""
+        _reject_naive(start_time=start_time, end_time=end_time)
         stream_key = (
             tuple((mid, tuple(cids)) for mid, cids in microgrid_components),
             tuple(metric.name for metric in metrics),
@@ -332,7 +357,7 @@ def stream_method() -> (
         return self._components_data_streams[stream_key].new_receiver()
 
     # pylint: disable=too-many-arguments
-    def receive_single_sensor_data(
+    def receive_single_sensor_data(  # noqa: DOC502
         self,
         *,
         microgrid_id: int,
@@ -356,6 +381,9 @@ def receive_single_sensor_data(
 
         Returns:
             A receiver of `MetricSample`s.
+
+        Raises:
+            ValueError: If start_time or end_time is timezone-naive.
         """
         receiver = self._receive_microgrid_sensors_data_batch(
             microgrid_sensors=[(microgrid_id, [sensor_id])],
@@ -368,7 +396,7 @@ def receive_single_sensor_data(
         return BatchUnrollReceiver(receiver)
 
     # pylint: disable=too-many-arguments
-    def receive_microgrid_sensors_data(
+    def receive_microgrid_sensors_data(  # noqa: DOC502
         self,
         *,
         microgrid_sensors: list[tuple[int, list[int]]],
@@ -391,6 +419,9 @@ def receive_microgrid_sensors_data(
 
         Returns:
             A receiver of `MetricSample`s.
+
+        Raises:
+            ValueError: If start_time or end_time is timezone-naive.
         """
         receiver = self._receive_microgrid_sensors_data_batch(
             microgrid_sensors=microgrid_sensors,
@@ -427,6 +458,7 @@ def _receive_microgrid_sensors_data_batch(
         Returns:
             A receiver of `SensorsDataBatch`s.
         """
+        _reject_naive(start_time=start_time, end_time=end_time)
         stream_key = (
             tuple((mid, tuple(sids)) for mid, sids in microgrid_sensors),
             tuple(metric.name for metric in metrics),
@@ -538,8 +570,10 @@ def receive_aggregated_data(
             A receiver of `MetricSample`s.
 
         Raises:
-            ValueError: If the resampling_period is not provided.
+            ValueError: If the resampling_period is not provided, or if
+                start_time or end_time is timezone-naive.
         """
+        _reject_naive(start_time=start_time, end_time=end_time)
         stream_key = (
             microgrid_id,
             metric.name,

src/frequenz/client/reporting/cli/__main__.py

@@ -16,6 +16,20 @@
 from frequenz.client.reporting._types import MetricSample
 
 
+def _aware_datetime(value: str) -> datetime:
+    """Parse an ISO datetime, rejecting naive input.
+
+    A naive datetime is ambiguous: the wire encoding assumes UTC while
+    datetime.timestamp() assumes local time.
+    """
+    parsed = datetime.fromisoformat(value)
+    if parsed.tzinfo is None:
+        raise argparse.ArgumentTypeError(
+            "datetime must include a timezone offset, e.g. 2024-05-01T00:00:00+00:00"
+        )
+    return parsed
+
+
 def main() -> None:
     """Parse arguments and run the client."""
     parser = argparse.ArgumentParser()
@@ -58,15 +72,15 @@ def main() -> None:
     )
     parser.add_argument(
         "--start",
-        type=datetime.fromisoformat,
-        help="Start datetime in YYYY-MM-DDTHH:MM:SS format",
+        type=_aware_datetime,
+        help="Start datetime in ISO format with a timezone offset",
         required=False,
         default=None,
     )
     parser.add_argument(
         "--end",
-        type=datetime.fromisoformat,
-        help="End datetime in YYYY-MM-DDTHH:MM:SS format",
+        type=_aware_datetime,
+        help="End datetime in ISO format with a timezone offset",
         required=False,
         default=None,
     )
@@ -93,6 +107,13 @@ def main() -> None:
         default=None,
     )
     args = parser.parse_args()
+    # An inverted window is a legal but empty range; warn since at the CLI it is
+    # almost always a typo.
+    if args.start and args.end and args.end < args.start:
+        print(
+            "warning: --end is before --start; no data will be returned",
+            file=sys.stderr,
+        )
     asyncio.run(
         run(
             microgrid_id=args.mid,

tests/test_client_reporting.py

@@ -3,14 +3,17 @@
 
 """Tests for the frequenz.client.reporting package."""
 
+from datetime import datetime, timezone
 from unittest.mock import MagicMock, patch
 
 import pytest
 from frequenz.api.reporting.v1alpha10.reporting_pb2_grpc import ReportingStub
 from frequenz.client.base.channel import ChannelOptions
 from frequenz.client.base.client import BaseApiClient
+from frequenz.client.common.metrics import Metric
 
 from frequenz.client.reporting import ReportingApiClient
+from frequenz.client.reporting._client import _reject_naive
 from frequenz.client.reporting._types import ComponentsDataBatch
 
 
@@ -42,6 +45,35 @@ async def test_client_initialization() -> None:
         )
 
 
+def test_reject_naive_passes_aware_and_none() -> None:
+    """Test that aware datetimes and None are accepted."""
+    _reject_naive(
+        start_time=datetime(2025, 1, 1, tzinfo=timezone.utc),
+        end_time=None,
+    )
+
+
+@pytest.mark.parametrize("name", ["start_time", "end_time"])
+def test_reject_naive_raises_on_naive(name: str) -> None:
+    """Test that a naive datetime is rejected, naming the offending parameter."""
+    with pytest.raises(ValueError, match=name):
+        _reject_naive(**{name: datetime(2025, 1, 1)})  # noqa: DTZ001
+
+
+def test_public_method_rejects_naive() -> None:
+    """Test that a public request method rejects naive input before any I/O."""
+    with patch.object(BaseApiClient, "__init__", return_value=None):
+        client = ReportingApiClient("grpc://localhost:50051")
+    with pytest.raises(ValueError, match="start_time"):
+        client.receive_microgrid_components_data(
+            microgrid_components=[(1, [2])],
+            metrics=Metric.AC_POWER_ACTIVE,
+            start_time=datetime(2025, 1, 1),  # noqa: DTZ001
+            end_time=None,
+            resampling_period=None,
+        )
+
+
 def test_components_data_batch_is_empty_true() -> None:
     """Test that the is_empty method returns True when the page is empty."""
     data_pb = MagicMock()