Skip to content

Commit cd53b73

Browse files
committed
PR Feedback
1 parent e8186e7 commit cd53b73

4 files changed

Lines changed: 89 additions & 12 deletions

File tree

durabletask/scheduled/client.py

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -108,10 +108,11 @@ def get_schedule(self, schedule_id: str) -> ScheduleDescription | None:
108108
except ScheduleNotFoundError:
109109
return None
110110

111-
def list_schedules(self, filter: ScheduleQuery | None = None) -> list[ScheduleDescription]:
111+
def list_schedules(self, schedule_query: ScheduleQuery | None = None) -> list[ScheduleDescription]:
112112
"""List schedules matching the given filter criteria."""
113-
prefix = filter.schedule_id_prefix if filter and filter.schedule_id_prefix else ""
114-
page_size = filter.page_size if filter and filter.page_size else ScheduleQuery.DEFAULT_PAGE_SIZE
113+
prefix = schedule_query.schedule_id_prefix if schedule_query and schedule_query.schedule_id_prefix else ""
114+
page_size = (schedule_query.page_size if schedule_query and schedule_query.page_size
115+
else ScheduleQuery.DEFAULT_PAGE_SIZE)
115116
query = EntityQuery(
116117
instance_id_starts_with=f"@{ENTITY_NAME}@{prefix}",
117118
include_state=True,
@@ -122,20 +123,20 @@ def list_schedules(self, filter: ScheduleQuery | None = None) -> list[ScheduleDe
122123
state = metadata.get_typed_state(ScheduleState)
123124
if state is None or state.schedule_configuration is None:
124125
continue
125-
if not self._matches_filter(state, filter):
126+
if not self._matches_filter(state, schedule_query):
126127
continue
127128
results.append(state.to_description())
128129
return results
129130

130131
@staticmethod
131-
def _matches_filter(state: ScheduleState, filter: ScheduleQuery | None) -> bool:
132-
if filter is None:
132+
def _matches_filter(state: ScheduleState, schedule_query: ScheduleQuery | None) -> bool:
133+
if schedule_query is None:
133134
return True
134-
if filter.status is not None and state.status != filter.status:
135+
if schedule_query.status is not None and state.status != schedule_query.status:
135136
return False
136137
created_at = state.schedule_created_at
137-
if filter.created_from is not None and not (created_at and created_at > filter.created_from):
138+
if schedule_query.created_from is not None and not (created_at and created_at > schedule_query.created_from):
138139
return False
139-
if filter.created_to is not None and not (created_at and created_at < filter.created_to):
140+
if schedule_query.created_to is not None and not (created_at and created_at < schedule_query.created_to):
140141
return False
141142
return True

durabletask/scheduled/registration.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,7 @@
22
# Licensed under the MIT License.
33

44
from durabletask.worker import TaskHubGrpcWorker
5-
from durabletask.scheduled.orchestrator import \
6-
execute_schedule_operation_orchestrator
5+
from durabletask.scheduled.orchestrator import execute_schedule_operation_orchestrator
76
from durabletask.scheduled.schedule_entity import ENTITY_NAME, Schedule
87

98

durabletask/testing/in_memory_backend.py

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,13 @@ def __init__(self, max_history_size: int = 10000, port: int = 50051):
153153
self._logger = logging.getLogger(__name__)
154154
self._shutdown_event = threading.Event()
155155
self._work_available = threading.Event()
156+
# Monotonic lifecycle counter, bumped on every stop()/reset(). Background
157+
# timers (e.g. delayed entity signals) capture it when scheduled and bail
158+
# if it has changed by the time they fire, so a timer created before a
159+
# stop/reset cannot mutate a subsequently-restarted or cleared backend.
160+
# Unlike ``_shutdown_event`` (which reset() clears to allow restart), this
161+
# only ever moves forward, so it reliably invalidates stale timers.
162+
self._generation = 0
156163

157164
def start(self) -> str:
158165
"""
@@ -183,6 +190,7 @@ def stop(self, grace: float | None = None):
183190
"""
184191
self._shutdown_event.set()
185192
self._work_available.set() # Unblock GetWorkItems loops
193+
self._generation += 1
186194
if self._server:
187195
stop_future = self._server.stop(grace)
188196
stop_future.wait()
@@ -208,6 +216,7 @@ def reset(self):
208216
self._state_waiters.clear()
209217
self._shutdown_event.clear()
210218
self._work_available.clear()
219+
self._generation += 1
211220

212221
# gRPC Service Methods
213222

@@ -1799,11 +1808,14 @@ def _schedule_delayed_entity_operation(self, entity_id: str, event: pb.HistoryEv
17991808
in-memory backend.
18001809
"""
18011810
delay = max(0.0, (fire_at - datetime.now(timezone.utc)).total_seconds())
1811+
# Capture the lifecycle generation so a timer outliving a stop()/reset()
1812+
# does not enqueue into a restarted or cleared backend.
1813+
scheduled_generation = self._generation
18021814

18031815
def fire() -> None:
18041816
time.sleep(delay)
18051817
with self._lock:
1806-
if self._shutdown_event.is_set():
1818+
if self._shutdown_event.is_set() or self._generation != scheduled_generation:
18071819
return
18081820
self._queue_entity_operation(entity_id, event)
18091821

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
# Copyright (c) Microsoft Corporation.
2+
# Licensed under the MIT License.
3+
4+
"""Unit tests for delayed-signal datetime normalization and in-memory backend
5+
timer lifecycle (regression coverage for PR #160 review feedback)."""
6+
7+
import time
8+
from datetime import datetime, timedelta, timezone
9+
10+
import durabletask.internal.orchestrator_service_pb2 as pb
11+
from durabletask.entities import EntityInstanceId
12+
from durabletask.internal import helpers
13+
from durabletask.internal.client_helpers import build_signal_entity_req
14+
from durabletask.testing import create_test_backend
15+
16+
from tests.durabletask._port_utils import find_free_port
17+
18+
19+
class TestSignalTimeNormalization:
20+
def test_build_signal_entity_req_naive_matches_aware(self):
21+
entity_id = EntityInstanceId("Recorder", "k")
22+
naive = datetime(2030, 1, 1, 12, 0, 0)
23+
aware = datetime(2030, 1, 1, 12, 0, 0, tzinfo=timezone.utc)
24+
25+
req_naive = build_signal_entity_req(entity_id, "ping", signal_time=naive)
26+
req_aware = build_signal_entity_req(entity_id, "ping", signal_time=aware)
27+
28+
assert req_naive.scheduledTime.seconds == req_aware.scheduledTime.seconds
29+
30+
def test_new_signal_entity_action_naive_matches_aware(self):
31+
entity_id = EntityInstanceId("Recorder", "k")
32+
naive = datetime(2030, 1, 1, 12, 0, 0)
33+
aware = datetime(2030, 1, 1, 12, 0, 0, tzinfo=timezone.utc)
34+
35+
a_naive = helpers.new_signal_entity_action(1, entity_id, "ping", None, "r1", naive)
36+
a_aware = helpers.new_signal_entity_action(1, entity_id, "ping", None, "r2", aware)
37+
38+
naive_ts = a_naive.sendEntityMessage.entityOperationSignaled.scheduledTime
39+
aware_ts = a_aware.sendEntityMessage.entityOperationSignaled.scheduledTime
40+
assert naive_ts.seconds == aware_ts.seconds
41+
42+
43+
class TestDelayedTimerLifecycle:
44+
def test_delayed_operation_does_not_fire_into_reset_backend(self):
45+
backend = create_test_backend(port=find_free_port())
46+
try:
47+
entity_id = "@recorder@k"
48+
event = pb.HistoryEvent(
49+
eventId=-1,
50+
entityOperationSignaled=pb.EntityOperationSignaledEvent(operation="ping"),
51+
)
52+
# Schedule the op to fire shortly in the future, then reset before it
53+
# fires so the timer wakes into a reset (new-generation) backend.
54+
fire_at = datetime.now(timezone.utc) + timedelta(seconds=0.3)
55+
backend._schedule_delayed_entity_operation( # pyright: ignore[reportPrivateUsage]
56+
entity_id, event, fire_at)
57+
backend.reset()
58+
59+
# Give the timer thread time to wake; it must observe the new
60+
# generation and refuse to recreate state in the reset backend.
61+
time.sleep(0.6)
62+
assert backend._entities == {} # pyright: ignore[reportPrivateUsage]
63+
finally:
64+
backend.stop()
65+
backend.reset()

0 commit comments

Comments
 (0)