Skip to content

Commit b811e1f

Browse files
fix(python): coerce dict retry config to Retry instance, fixing possible registration crash (#199)
GitOrigin-RevId: 3a493d288b2e428ee51e777dae3b62a5406f1ef5
1 parent 9429f48 commit b811e1f

3 files changed

Lines changed: 102 additions & 0 deletions

File tree

python/render_sdk/workflows/task.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,14 @@ class Retry:
2323
wait_duration_ms: int
2424
backoff_scaling: float = 1.5
2525

26+
@classmethod
27+
def from_dict(cls, d: dict) -> "Retry":
28+
return cls(
29+
max_retries=d["max_retries"],
30+
wait_duration_ms=d["wait_duration_ms"],
31+
backoff_scaling=d["backoff_scaling"],
32+
)
33+
2634

2735
@dataclass
2836
class Options:
@@ -39,6 +47,10 @@ class Options:
3947
timeout_seconds: int | None = None
4048
plan: str | None = None
4149

50+
def __post_init__(self):
51+
if isinstance(self.retry, dict):
52+
self.retry = Retry.from_dict(self.retry)
53+
4254

4355
@dataclass
4456
class ParameterInfo:

python/render_sdk/workflows/tests/test_end_to_end.py

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,77 @@ def timeout_and_retry_task(x: int) -> int:
190190
assert combined_payload.options.retry.max_retries == 2
191191

192192

193+
def test_task_registration_with_dict_retry_config(
194+
task_registry, task_decorator, mocker
195+
):
196+
"""
197+
Test that registration handles dict retry configs passed to Options().
198+
A caller may construct Options(retry={...}) with a plain dict instead
199+
of a Retry instance. Options.__post_init__ should coerce it.
200+
"""
201+
mock_uds_client_class = mocker.patch("render_sdk.workflows.runner.UDSClient")
202+
mock_client_instance = mocker.Mock()
203+
mock_uds_client_class.return_value = mock_client_instance
204+
mock_client_instance.register_tasks = mocker.AsyncMock(
205+
return_value={"status": "success"},
206+
)
207+
mock_client_instance.disconnect = mocker.AsyncMock()
208+
209+
# Options receives a plain dict instead of a Retry instance
210+
dict_retry = {
211+
"max_retries": 5,
212+
"wait_duration_ms": 2000,
213+
"backoff_scaling": 2.0,
214+
}
215+
options = Options(retry=dict_retry)
216+
217+
@task_decorator(options=options)
218+
def my_task(x: int) -> int:
219+
return x
220+
221+
mock_get_registry = mocker.patch("render_sdk.workflows.runner.get_task_registry")
222+
mock_get_registry.return_value = task_registry
223+
224+
register("/tmp/test.sock") # noqa:S108
225+
226+
sent_tasks = mock_client_instance.register_tasks.call_args[0][0]
227+
task_payload = sent_tasks.tasks[0]
228+
229+
assert task_payload.options.retry.max_retries == 5
230+
assert task_payload.options.retry.wait_duration_ms == 2000
231+
assert task_payload.options.retry.factor == 2.0
232+
233+
234+
def test_task_registration_without_retry_config(task_registry, task_decorator, mocker):
235+
"""
236+
Test that tasks with no retry config register successfully. This is the
237+
common case that was unaffected by the dict retry bug.
238+
"""
239+
from render_sdk.workflows.callback_api.types import UNSET
240+
241+
mock_uds_client_class = mocker.patch("render_sdk.workflows.runner.UDSClient")
242+
mock_client_instance = mocker.Mock()
243+
mock_uds_client_class.return_value = mock_client_instance
244+
mock_client_instance.register_tasks = mocker.AsyncMock(
245+
return_value={"status": "success"},
246+
)
247+
mock_client_instance.disconnect = mocker.AsyncMock()
248+
249+
@task_decorator
250+
def my_task(x: int) -> int:
251+
return x
252+
253+
mock_get_registry = mocker.patch("render_sdk.workflows.runner.get_task_registry")
254+
mock_get_registry.return_value = task_registry
255+
256+
register("/tmp/test.sock") # noqa:S108
257+
258+
sent_tasks = mock_client_instance.register_tasks.call_args[0][0]
259+
task_payload = sent_tasks.tasks[0]
260+
261+
assert task_payload.options.retry is UNSET
262+
263+
193264
@pytest.mark.asyncio
194265
async def test_callback_payloads_with_mocked_client(
195266
task_registry,

python/render_sdk/workflows/tests/test_registration.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,25 @@ def task_with_retry_only(x: int) -> int:
119119
assert retry_task.options.retry.max_retries == 1
120120

121121

122+
def test_options_coerces_dict_retry_to_retry():
123+
"""
124+
Test that Options.__post_init__ coerces a dict retry config to a Retry instance.
125+
"""
126+
options = Options(
127+
retry={"max_retries": 5, "wait_duration_ms": 2000, "backoff_scaling": 2.0}
128+
)
129+
assert isinstance(options.retry, Retry)
130+
assert options.retry.max_retries == 5
131+
assert options.retry.wait_duration_ms == 2000
132+
assert options.retry.backoff_scaling == 2.0
133+
134+
135+
def test_options_rejects_incomplete_dict_retry():
136+
"""Test that dict-to-Retry coercion raises KeyError for missing required keys."""
137+
with pytest.raises(KeyError):
138+
Options(retry={})
139+
140+
122141
def test_task_registration_preserves_function_attributes(task_registry, task_decorator):
123142
"""Test that task registration preserves original function attributes."""
124143

0 commit comments

Comments
 (0)