Skip to content

Commit 3490587

Browse files
fgogollifgogolli
committed
fix: RequestStatusError crash and SDK pagination kwargs dropped
- get_request_status: replace RequestStatusError dataclass append with plain dict so downstream .get() callers don't crash on error path - sdk.list_requests: forward offset kwarg to ListRequestsInput - sdk.list_machines: forward limit and offset kwargs to ListMachinesInput - Add 4 tests for error path type contract in test_get_request_status_orchestrator - Add 8 tests for pagination forwarding in test_sdk_request_methods
1 parent b6b6c4f commit 3490587

4 files changed

Lines changed: 237 additions & 4 deletions

File tree

‎src/orb/application/services/orchestration/get_request_status.py‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
from orb.application.services.orchestration.dtos import (
1010
GetRequestStatusInput,
1111
GetRequestStatusOutput,
12-
RequestStatusError,
1312
)
1413
from orb.domain.base.ports.logging_port import LoggingPort
1514

@@ -43,7 +42,7 @@ async def execute(self, input: GetRequestStatusInput) -> GetRequestStatusOutput:
4342
request_dicts.append(self._to_dict(result))
4443
except Exception as exc:
4544
self._logger.error("Failed to get status for %s: %s", request_id, exc)
46-
request_dicts.append(RequestStatusError(request_id=request_id, error=str(exc)))
45+
request_dicts.append({"request_id": request_id, "error": str(exc)})
4746

4847
return GetRequestStatusOutput(requests=request_dicts)
4948

‎src/orb/sdk/client.py‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -514,6 +514,7 @@ async def list_requests(self, **kwargs) -> dict:
514514
ListRequestsInput(
515515
status=kwargs.get("status"),
516516
limit=kwargs.get("limit", 50),
517+
offset=kwargs.get("offset", 0),
517518
sync=kwargs.get("sync", False),
518519
)
519520
)
@@ -589,6 +590,8 @@ async def list_machines(self, **kwargs) -> dict:
589590
status=kwargs.get("status"),
590591
provider_name=kwargs.get("provider_name"),
591592
request_id=kwargs.get("request_id"),
593+
limit=kwargs.get("limit", 100),
594+
offset=kwargs.get("offset", 0),
592595
)
593596
)
594597
scheduler = self._container.get_optional(SchedulerPort)

‎tests/unit/application/services/orchestration/test_get_request_status_orchestrator.py‎

Lines changed: 39 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,8 +118,8 @@ async def test_execute_query_error_returns_error_dict(self, orchestrator, mock_q
118118
result = await orchestrator.execute(input)
119119
assert len(result.requests) == 1
120120
entry = result.requests[0]
121-
assert entry.request_id == "req-bad"
122-
assert entry.error == "not found"
121+
assert entry["request_id"] == "req-bad"
122+
assert entry["error"] == "not found"
123123

124124
@pytest.mark.asyncio
125125
async def test_execute_query_error_logs_error(self, orchestrator, mock_query_bus, mock_logger):
@@ -176,3 +176,40 @@ async def test_detailed_true_result_contains_expected_keys(self, orchestrator, m
176176
assert entry["request_id"] == "req-detail-1"
177177
assert entry["status"] == "running"
178178
assert entry["machine_references"] == ["m-001", "m-002"]
179+
180+
@pytest.mark.asyncio
181+
async def test_execute_query_error_entry_is_dict(self, orchestrator, mock_query_bus):
182+
mock_query_bus.execute.side_effect = Exception("boom")
183+
input = GetRequestStatusInput(request_ids=["req-bad"])
184+
result = await orchestrator.execute(input)
185+
assert isinstance(result.requests[0], dict)
186+
187+
@pytest.mark.asyncio
188+
async def test_execute_query_error_entry_has_request_id_and_error_keys(
189+
self, orchestrator, mock_query_bus
190+
):
191+
mock_query_bus.execute.side_effect = Exception("boom")
192+
input = GetRequestStatusInput(request_ids=["req-bad"])
193+
result = await orchestrator.execute(input)
194+
assert result.requests[0].get("request_id") == "req-bad"
195+
196+
@pytest.mark.asyncio
197+
async def test_execute_query_error_entry_get_status_returns_empty_string(
198+
self, orchestrator, mock_query_bus
199+
):
200+
mock_query_bus.execute.side_effect = Exception("boom")
201+
input = GetRequestStatusInput(request_ids=["req-bad"])
202+
result = await orchestrator.execute(input)
203+
assert result.requests[0].get("status", "") == ""
204+
205+
@pytest.mark.asyncio
206+
async def test_execute_mixed_success_and_error_all_entries_are_dicts(
207+
self, orchestrator, mock_query_bus
208+
):
209+
ok = MagicMock(spec=["model_dump"])
210+
ok.model_dump = MagicMock(return_value={"request_id": "req-ok", "status": "running"})
211+
mock_query_bus.execute.side_effect = [ok, Exception("fail")]
212+
input = GetRequestStatusInput(request_ids=["req-ok", "req-bad"])
213+
result = await orchestrator.execute(input)
214+
assert len(result.requests) == 2
215+
assert all(isinstance(e, dict) for e in result.requests)

‎tests/unit/sdk/test_sdk_request_methods.py‎

Lines changed: 194 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,194 @@
1+
"""Unit tests for orchestrator-backed request and machine list methods on ORBClient."""
2+
3+
from unittest.mock import AsyncMock, MagicMock
4+
5+
import pytest
6+
7+
from orb.sdk.client import ORBClient
8+
from orb.sdk.exceptions import SDKError
9+
10+
# ---------------------------------------------------------------------------
11+
# Helpers
12+
# ---------------------------------------------------------------------------
13+
14+
15+
def _initialized_sdk() -> ORBClient:
16+
sdk = ORBClient(config={"provider": "aws"})
17+
sdk._initialized = True
18+
sdk._container = MagicMock()
19+
return sdk
20+
21+
22+
def _mock_container(sdk: ORBClient, orchestrator_class, orchestrator, scheduler=None):
23+
"""Wire container.get() for one orchestrator and container.get_optional() for scheduler."""
24+
25+
def _get(cls):
26+
if cls is orchestrator_class:
27+
return orchestrator
28+
raise KeyError(cls)
29+
30+
sdk._container.get.side_effect = _get
31+
sdk._container.get_optional.return_value = scheduler
32+
33+
34+
# ---------------------------------------------------------------------------
35+
# list_requests
36+
# ---------------------------------------------------------------------------
37+
38+
39+
class TestListRequests:
40+
@pytest.mark.asyncio
41+
@pytest.mark.unit
42+
async def test_list_requests_forwards_offset(self):
43+
from orb.application.services.orchestration.dtos import (
44+
ListRequestsInput,
45+
ListRequestsOutput,
46+
)
47+
from orb.application.services.orchestration.list_requests import (
48+
ListRequestsOrchestrator,
49+
)
50+
51+
mock_orch = MagicMock()
52+
mock_orch.execute = AsyncMock(return_value=ListRequestsOutput(requests=[]))
53+
54+
sdk = _initialized_sdk()
55+
_mock_container(sdk, ListRequestsOrchestrator, mock_orch)
56+
57+
await sdk.list_requests(offset=5)
58+
59+
mock_orch.execute.assert_called_once()
60+
call_input: ListRequestsInput = mock_orch.execute.call_args[0][0]
61+
assert call_input.offset == 5
62+
63+
@pytest.mark.asyncio
64+
@pytest.mark.unit
65+
async def test_list_requests_default_offset_is_zero(self):
66+
from orb.application.services.orchestration.dtos import (
67+
ListRequestsInput,
68+
ListRequestsOutput,
69+
)
70+
from orb.application.services.orchestration.list_requests import (
71+
ListRequestsOrchestrator,
72+
)
73+
74+
mock_orch = MagicMock()
75+
mock_orch.execute = AsyncMock(return_value=ListRequestsOutput(requests=[]))
76+
77+
sdk = _initialized_sdk()
78+
_mock_container(sdk, ListRequestsOrchestrator, mock_orch)
79+
80+
await sdk.list_requests()
81+
82+
call_input: ListRequestsInput = mock_orch.execute.call_args[0][0]
83+
assert call_input.offset == 0
84+
85+
@pytest.mark.asyncio
86+
@pytest.mark.unit
87+
async def test_list_requests_forwards_offset_and_limit(self):
88+
from orb.application.services.orchestration.dtos import (
89+
ListRequestsInput,
90+
ListRequestsOutput,
91+
)
92+
from orb.application.services.orchestration.list_requests import (
93+
ListRequestsOrchestrator,
94+
)
95+
96+
mock_orch = MagicMock()
97+
mock_orch.execute = AsyncMock(return_value=ListRequestsOutput(requests=[]))
98+
99+
sdk = _initialized_sdk()
100+
_mock_container(sdk, ListRequestsOrchestrator, mock_orch)
101+
102+
await sdk.list_requests(offset=5, limit=10)
103+
104+
call_input: ListRequestsInput = mock_orch.execute.call_args[0][0]
105+
assert call_input.offset == 5
106+
assert call_input.limit == 10
107+
108+
@pytest.mark.asyncio
109+
@pytest.mark.unit
110+
async def test_list_requests_not_initialized_raises(self):
111+
sdk = ORBClient(config={"provider": "aws"})
112+
with pytest.raises(SDKError):
113+
await sdk.list_requests()
114+
115+
116+
# ---------------------------------------------------------------------------
117+
# list_machines
118+
# ---------------------------------------------------------------------------
119+
120+
121+
class TestListMachines:
122+
@pytest.mark.asyncio
123+
@pytest.mark.unit
124+
async def test_list_machines_forwards_offset(self):
125+
from orb.application.services.orchestration.dtos import (
126+
ListMachinesInput,
127+
ListMachinesOutput,
128+
)
129+
from orb.application.services.orchestration.list_machines import (
130+
ListMachinesOrchestrator,
131+
)
132+
133+
mock_orch = MagicMock()
134+
mock_orch.execute = AsyncMock(return_value=ListMachinesOutput(machines=[]))
135+
136+
sdk = _initialized_sdk()
137+
_mock_container(sdk, ListMachinesOrchestrator, mock_orch)
138+
139+
await sdk.list_machines(offset=10)
140+
141+
call_input: ListMachinesInput = mock_orch.execute.call_args[0][0]
142+
assert call_input.offset == 10
143+
144+
@pytest.mark.asyncio
145+
@pytest.mark.unit
146+
async def test_list_machines_forwards_limit(self):
147+
from orb.application.services.orchestration.dtos import (
148+
ListMachinesInput,
149+
ListMachinesOutput,
150+
)
151+
from orb.application.services.orchestration.list_machines import (
152+
ListMachinesOrchestrator,
153+
)
154+
155+
mock_orch = MagicMock()
156+
mock_orch.execute = AsyncMock(return_value=ListMachinesOutput(machines=[]))
157+
158+
sdk = _initialized_sdk()
159+
_mock_container(sdk, ListMachinesOrchestrator, mock_orch)
160+
161+
await sdk.list_machines(limit=25)
162+
163+
call_input: ListMachinesInput = mock_orch.execute.call_args[0][0]
164+
assert call_input.limit == 25
165+
166+
@pytest.mark.asyncio
167+
@pytest.mark.unit
168+
async def test_list_machines_default_offset_and_limit(self):
169+
from orb.application.services.orchestration.dtos import (
170+
ListMachinesInput,
171+
ListMachinesOutput,
172+
)
173+
from orb.application.services.orchestration.list_machines import (
174+
ListMachinesOrchestrator,
175+
)
176+
177+
mock_orch = MagicMock()
178+
mock_orch.execute = AsyncMock(return_value=ListMachinesOutput(machines=[]))
179+
180+
sdk = _initialized_sdk()
181+
_mock_container(sdk, ListMachinesOrchestrator, mock_orch)
182+
183+
await sdk.list_machines()
184+
185+
call_input: ListMachinesInput = mock_orch.execute.call_args[0][0]
186+
assert call_input.offset == 0
187+
assert call_input.limit == 100
188+
189+
@pytest.mark.asyncio
190+
@pytest.mark.unit
191+
async def test_list_machines_not_initialized_raises(self):
192+
sdk = ORBClient(config={"provider": "aws"})
193+
with pytest.raises(SDKError):
194+
await sdk.list_machines()

0 commit comments

Comments
 (0)