diff --git a/RELEASE_NOTES.md b/RELEASE_NOTES.md index 3b27ef26..33b16cfa 100644 --- a/RELEASE_NOTES.md +++ b/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. diff --git a/src/frequenz/client/dispatch/__main__.py b/src/frequenz/client/dispatch/__main__.py index 47e76c4e..1612c41f 100644 --- a/src/frequenz/client/dispatch/__main__.py +++ b/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), diff --git a/src/frequenz/client/dispatch/_internal_types.py b/src/frequenz/client/dispatch/_internal_types.py index b17d0fbb..4a48e3d2 100644 --- a/src/frequenz/client/dispatch/_internal_types.py +++ b/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, diff --git a/src/frequenz/client/dispatch/test/_service.py b/src/frequenz/client/dispatch/test/_service.py index 2a9bc8b9..27e8fbda 100644 --- a/src/frequenz/client/dispatch/test/_service.py +++ b/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( diff --git a/src/frequenz/client/dispatch/types.py b/src/frequenz/client/dispatch/types.py index 33d5c736..8d1daf89 100644 --- a/src/frequenz/client/dispatch/types.py +++ b/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, diff --git a/tests/test_client.py b/tests/test_client.py index 57c82c86..e7fff5bb 100644 --- a/tests/test_client.py +++ b/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: