Skip to content

Commit b02ebbf

Browse files
committed
fix(workflow): treat RetryConfig(max_attempts=0) as no retries
RetryConfig.max_attempts documents "If 0 or 1, it means no retries. If not specified, default to 5." and defaults to None. _should_retry_node resolved the default with `retry_config.max_attempts or 5`, which treats an explicit 0 as falsy and replaces it with 5, so a node configured for no retries ran up to 5 attempts. Use an explicit None check so 0 and 1 disable retries while the unset (None) case still defaults to 5.
1 parent e6df097 commit b02ebbf

2 files changed

Lines changed: 37 additions & 1 deletion

File tree

src/google/adk/workflow/utils/_retry_utils.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,9 @@ def _should_retry_node(
3232
return False
3333

3434
attempt_count = node_state.attempt_count
35-
max_attempts = retry_config.max_attempts or 5
35+
max_attempts = (
36+
retry_config.max_attempts if retry_config.max_attempts is not None else 5
37+
)
3638

3739
# attempt_count starts at 1 for the original request.
3840
# So if attempt_count >= max_attempts, we have reached the limit.

tests/unittests/workflow/utils/test_retry_utils.py

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
from google.adk.workflow._node_state import NodeState
1818
from google.adk.workflow._retry_config import RetryConfig
1919
from google.adk.workflow.utils._retry_utils import _get_retry_delay
20+
from google.adk.workflow.utils._retry_utils import _should_retry_node
2021
import pytest
2122

2223

@@ -68,3 +69,36 @@ def test_adds_jitter_when_enabled(self):
6869

6970
assert all(5.0 <= d <= 15.0 for d in delays)
7071
assert len(set(delays)) > 1
72+
73+
74+
class TestShouldRetryNode:
75+
76+
def test_no_config_never_retries(self):
77+
"""Without a retry config, a node is never retried."""
78+
assert _should_retry_node(RuntimeError(), None, NodeState(attempt_count=1)) is False
79+
80+
@pytest.mark.parametrize("max_attempts", [0, 1])
81+
def test_max_attempts_zero_or_one_disables_retries(self, max_attempts):
82+
"""max_attempts of 0 or 1 means no retries (per RetryConfig docs).
83+
84+
A falsy-coalescing default (``max_attempts or 5``) wrongly treated an
85+
explicit ``0`` as unset and allowed 5 attempts.
86+
"""
87+
config = RetryConfig(max_attempts=max_attempts)
88+
89+
assert _should_retry_node(RuntimeError(), config, NodeState(attempt_count=1)) is False
90+
91+
def test_retries_until_max_attempts(self):
92+
"""A node is retried while attempt_count is below max_attempts."""
93+
config = RetryConfig(max_attempts=3)
94+
95+
assert _should_retry_node(RuntimeError(), config, NodeState(attempt_count=1)) is True
96+
assert _should_retry_node(RuntimeError(), config, NodeState(attempt_count=2)) is True
97+
assert _should_retry_node(RuntimeError(), config, NodeState(attempt_count=3)) is False
98+
99+
def test_unset_max_attempts_defaults_to_five(self):
100+
"""When max_attempts is unset (None), the default of 5 applies."""
101+
config = RetryConfig()
102+
103+
assert _should_retry_node(RuntimeError(), config, NodeState(attempt_count=4)) is True
104+
assert _should_retry_node(RuntimeError(), config, NodeState(attempt_count=5)) is False

0 commit comments

Comments
 (0)