Skip to content

Commit 344df10

Browse files
committed
fix: Let every device settle before a bulk failure surfaces
Applying config across devices used a plain gather, so one device raising propagated immediately while its siblings carried on issuing SetConfig to real hardware with nobody awaiting them. That is the same hazard the capture path already guarded against, and it matters more here because these calls write. Both paths now share one helper that runs every device to completion before surfacing the first failure.
1 parent 0a7bea7 commit 344df10

2 files changed

Lines changed: 52 additions & 27 deletions

File tree

packages/core/src/core/use_cases/bulk_operations.py

Lines changed: 27 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,16 @@
11
import asyncio
2+
from collections.abc import Coroutine, Iterable
23
from datetime import UTC, datetime
3-
from typing import Any
4+
from typing import Any, TypeVar
45

56
from ..domain.entities.config_snapshot import DeviceSnapshot
67
from ..domain.entities.exceptions import BulkOperationError
78
from ..domain.value_objects.action_result import ActionResult
89
from ..gateways.device import DeviceGateway
910
from .capture_device_config import CaptureDeviceConfig
1011

12+
T = TypeVar("T")
13+
1114

1215
def _unique(device_ips: list[str]) -> list[str]:
1316
"""The given IPs in order, without repeats.
@@ -20,6 +23,23 @@ def _unique(device_ips: list[str]) -> list[str]:
2023
return list(dict.fromkeys(device_ips))
2124

2225

26+
async def _gather_settled(coroutines: Iterable[Coroutine[Any, Any, T]]) -> list[T]:
27+
"""Run every device to completion, in order, then surface the first failure.
28+
29+
Plain ``asyncio.gather`` raises the moment one device fails and does not
30+
cancel its siblings, leaving them issuing requests to real hardware with
31+
nobody awaiting them. Letting every task settle first keeps one bad device
32+
from doing that.
33+
"""
34+
results = await asyncio.gather(*coroutines, return_exceptions=True)
35+
settled: list[T] = []
36+
for result in results:
37+
if isinstance(result, BaseException):
38+
raise result
39+
settled.append(result)
40+
return settled
41+
42+
2343
class BulkOperationsUseCase:
2444

2545
def __init__(
@@ -116,7 +136,9 @@ async def export_bulk_config(
116136
Dictionary containing export metadata and device configurations
117137
"""
118138
targets = _unique(device_ips)
119-
snapshots = await self._capture_all(targets, component_types)
139+
snapshots = await _gather_settled(
140+
self._capture_device(ip, component_types) for ip in targets
141+
)
120142

121143
return {
122144
"export_metadata": {
@@ -131,26 +153,6 @@ async def export_bulk_config(
131153
},
132154
}
133155

134-
async def _capture_all(
135-
self, device_ips: list[str], component_types: list[str]
136-
) -> list[DeviceSnapshot | None]:
137-
"""Capture every device concurrently, in the order they were asked for.
138-
139-
Gathering with ``return_exceptions`` lets every capture finish before a
140-
failure surfaces, so one bad device does not leave the others running
141-
against real hardware with nobody awaiting them.
142-
"""
143-
results = await asyncio.gather(
144-
*(self._capture_device(ip, component_types) for ip in device_ips),
145-
return_exceptions=True,
146-
)
147-
snapshots: list[DeviceSnapshot | None] = []
148-
for result in results:
149-
if isinstance(result, BaseException):
150-
raise result
151-
snapshots.append(result)
152-
return snapshots
153-
154156
async def _capture_device(
155157
self, device_ip: str, component_types: list[str]
156158
) -> DeviceSnapshot | None:
@@ -183,11 +185,9 @@ async def apply_bulk_config(
183185
Returns:
184186
List of action results
185187
"""
186-
per_device = await asyncio.gather(
187-
*(
188-
self._apply_device_config(device_ip, component_type, config)
189-
for device_ip in _unique(device_ips)
190-
)
188+
per_device = await _gather_settled(
189+
self._apply_device_config(device_ip, component_type, config)
190+
for device_ip in _unique(device_ips)
191191
)
192192
return [result for results in per_device for result in results]
193193

packages/core/tests/unit/use_cases/test_bulk_operations.py

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1013,3 +1013,28 @@ async def test_it_exports_a_repeated_device_only_once(
10131013

10141014
assert mock_device_gateway.get_device_status.await_count == 1
10151015
assert result["export_metadata"]["total_devices"] == 1
1016+
1017+
async def test_it_lets_every_apply_finish_before_a_failure_surfaces(
1018+
self, use_case, mock_device_gateway
1019+
):
1020+
# Same rule as the capture path: a device that raises must not leave its
1021+
# siblings still writing to hardware with nobody awaiting them.
1022+
finished = []
1023+
1024+
async def get_component_keys(device_ip, component_type):
1025+
if device_ip == "192.168.1.100":
1026+
raise RuntimeError("boom")
1027+
await asyncio.sleep(0.05)
1028+
finished.append(device_ip)
1029+
return []
1030+
1031+
mock_device_gateway.get_component_keys = AsyncMock(
1032+
side_effect=get_component_keys
1033+
)
1034+
1035+
with pytest.raises(RuntimeError, match="boom"):
1036+
await use_case.apply_bulk_config(
1037+
["192.168.1.100", "192.168.1.101"], "switch", {}
1038+
)
1039+
1040+
assert finished == ["192.168.1.101"]

0 commit comments

Comments
 (0)