Skip to content

Commit 8a81219

Browse files
committed
Fix exception when two dispatches have equal start time
The heapq will first look at the given time to compare, but when it is identical it will look at the attached dispatch objects for comparison. But those just aren't ready for so much responsibility. UNTIL NOW! Cudos to Merlin for finding this bug! Signed-off-by: Mathias L. Baumann <[email protected]>
1 parent 2f9c764 commit 8a81219

File tree

3 files changed

+43
-2
lines changed

3 files changed

+43
-2
lines changed

RELEASE_NOTES.md

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,16 @@
22

33
## Summary
44

5-
This is a hot fix for recurrence not working
5+
<!-- Here goes a general summary of what this release is about -->
6+
7+
## Upgrading
8+
9+
<!-- Here goes notes on how to upgrade from previous versions, including deprecations and what they should be replaced with -->
10+
11+
## New Features
12+
13+
<!-- Here goes the main new features and examples or instructions on how to use them -->
14+
15+
## Bug Fixes
16+
17+
* Fixed a crash in the `DispatchManagingActor` when dispatches shared an equal start time.

src/frequenz/dispatch/_dispatch.py

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
from dataclasses import dataclass
99
from datetime import datetime, timezone
1010
from enum import Enum
11-
from typing import Iterator, cast
11+
from typing import Any, Iterator, Self, cast
1212

1313
from dateutil import rrule
1414
from frequenz.client.dispatch.types import Dispatch as BaseDispatch
@@ -264,3 +264,14 @@ def _until(self, now: datetime) -> datetime | None:
264264
return None
265265

266266
return latest_past_start + self.duration
267+
268+
def __lt__(self, other: Self) -> Any:
269+
"""Compare two dispatches by their id.
270+
271+
Args:
272+
other: The other dispatch.
273+
274+
Returns:
275+
True if the current dispatch has a lower id than the other dispatch, False otherwise.
276+
"""
277+
return self.id < other.id

tests/test_mananging_actor.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
"""Test the dispatch runner."""
55

66
import asyncio
7+
import heapq
78
from dataclasses import dataclass, replace
89
from datetime import datetime, timedelta, timezone
910
from typing import AsyncIterator, Iterator
@@ -128,6 +129,23 @@ async def test_simple_start_stop(
128129
assert test_env.actor.is_running is False
129130

130131

132+
def test_heapq_dispatch_compare(test_env: TestEnv) -> None:
133+
"""Test that the heapq compare function works."""
134+
dispatch1 = test_env.generator.generate_dispatch()
135+
dispatch2 = test_env.generator.generate_dispatch()
136+
137+
# Simulate two dispatches with the same 'until' time
138+
now = datetime.now(timezone.utc)
139+
until_time = now + timedelta(minutes=5)
140+
141+
# Create the heap
142+
scheduled_events: list[tuple[datetime, Dispatch]] = []
143+
144+
# Push two events with the same 'until' time onto the heap
145+
heapq.heappush(scheduled_events, (until_time, Dispatch(dispatch1)))
146+
heapq.heappush(scheduled_events, (until_time, Dispatch(dispatch2)))
147+
148+
131149
async def test_dry_run(test_env: TestEnv, fake_time: time_machine.Coordinates) -> None:
132150
"""Test the dry run mode."""
133151
dispatch = test_env.generator.generate_dispatch()

0 commit comments

Comments
 (0)