Fix that duration=0 was sent & received as None (frequenz-floss#126)

Author: Marenz

RELEASE_NOTES.md

@@ -2,7 +2,4 @@
 
 ## Bug Fixes
 
-* Fix missing dependency in last release.
-* The `FakeClient.set_dispatches()` method now correctly updates `FakeService._last_id` which is used to generate unique dispatch IDs.
-* Fix that streams were globally shared between all clients.
-
+* Fix that duration=0 was sent & received as None.

src/frequenz/client/dispatch/__main__.py

@@ -128,10 +128,10 @@ def format_line(key: str, value: str, color: str = "cyan") -> str:
     lines.append(format_line("ID", str(dispatch.id)))
     lines.append(format_line("Type", str(dispatch.type)))
     lines.append(format_line("Start Time", format_datetime(dispatch.start_time)))
-    if dispatch.duration:
+    if dispatch.duration is not None:
         lines.append(format_line("Duration", str(dispatch.duration)))
     else:
-        lines.append(format_line("Duration", "Infinite"))
+        lines.append(format_line("Duration", "Indefinite"))
     lines.append(format_line("Target", target_str))
     lines.append(format_line("Active", str(dispatch.active)))
     lines.append(format_line("Dry Run", str(dispatch.dry_run)))
@@ -407,8 +407,7 @@ async def create(
     kwargs = {k: v for k, v in kwargs.items() if v is not None}
 
     # Required for client.create
-    if not kwargs.get("duration"):
-        kwargs["duration"] = None
+    kwargs.setdefault("duration", None)
 
     dispatch = await ctx.obj["client"].create(
         recurrence=parse_recurrence(kwargs),

src/frequenz/client/dispatch/_internal_types.py

@@ -125,7 +125,9 @@ def to_protobuf(self) -> PBDispatchCreateRequest:
                     else Timestamp()
                 ),
                 duration=(
-                    round(self.duration.total_seconds()) if self.duration else None
+                    None
+                    if self.duration is None
+                    else round(self.duration.total_seconds())
                 ),
                 target=_target_components_to_protobuf(self.target),
                 is_active=self.active,

src/frequenz/client/dispatch/test/_service.py

@@ -265,12 +265,18 @@ async def UpdateMicrogridDispatch(
 
             match split_path[0]:
                 # Fields that can be assigned directly
-                case "is_active" | "duration":
+                case "is_active":
                     setattr(
                         pb_dispatch.data,
                         split_path[0],
                         getattr(request.update, split_path[0]),
                     )
+                # Duration needs extra handling for clearing
+                case "duration":
+                    if request.update.HasField("duration"):
+                        pb_dispatch.data.duration = request.update.duration
+                    else:
+                        pb_dispatch.data.ClearField("duration")
                 # Fields that need to be copied
                 case "start_time" | "target" | "payload":
                     getattr(pb_dispatch.data, split_path[0]).CopyFrom(

src/frequenz/client/dispatch/types.py

@@ -293,7 +293,7 @@ def from_protobuf(cls, pb_object: PBDispatch) -> "Dispatch":
             start_time=to_datetime(pb_object.data.start_time),
             duration=(
                 timedelta(seconds=pb_object.data.duration)
-                if pb_object.data.duration
+                if pb_object.data.HasField("duration")
                 else None
             ),
             target=_target_components_from_protobuf(pb_object.data.target),
@@ -322,7 +322,9 @@ def to_protobuf(self) -> PBDispatch:
                 type=self.type,
                 start_time=to_timestamp(self.start_time),
                 duration=(
-                    round(self.duration.total_seconds()) if self.duration else None
+                    None
+                    if self.duration is None
+                    else round(self.duration.total_seconds())
                 ),
                 target=_target_components_to_protobuf(self.target),
                 is_active=self.active,

tests/test_client.py

@@ -74,6 +74,15 @@ async def test_create_duration_none(client: FakeClient, sample: Dispatch) -> Non
     assert dispatch == sample
 
 
+async def test_create_duration_0(client: FakeClient, sample: Dispatch) -> None:
+    """Test creating a dispatch with a 0 duration."""
+    microgrid_id = random.randint(1, 100)
+    sample = replace(sample, duration=timedelta(minutes=0))
+    dispatch = await client.create(**to_create_params(microgrid_id, sample))
+    sample = _update_metadata(sample, dispatch)
+    assert dispatch == sample
+
+
 async def test_list_dispatches(
     client: FakeClient, generator: DispatchGenerator
 ) -> None:
@@ -172,6 +181,24 @@ async def test_update_dispatch_to_no_duration(
     assert client.dispatches(microgrid_id)[0].duration is None
 
 
+async def test_update_dispatch_to_0_duration(
+    client: FakeClient, sample: Dispatch
+) -> None:
+    """Test updating the duration field of a dispatch to 0."""
+    microgrid_id = random.randint(1, 100)
+    client.set_dispatches(
+        microgrid_id=microgrid_id,
+        value=[replace(sample, duration=timedelta(minutes=10))],
+    )
+
+    await client.update(
+        microgrid_id=microgrid_id,
+        dispatch_id=sample.id,
+        new_fields={"duration": timedelta(minutes=0)},
+    )
+    assert client.dispatches(microgrid_id)[0].duration == timedelta(minutes=0)
+
+
 async def test_update_dispatch_from_no_duration(
     client: FakeClient, sample: Dispatch
 ) -> None: