Skip to content

Commit d74c157

Browse files
authored
[dashboards][core] Make do_reply accept status_code, instead of success: bool (#58384)
## Description Pass in `status_code` directly into `do_reply`. This is a follow up to #58255 ## Related issues ## Additional information --------- Signed-off-by: iamjustinhsu <[email protected]>
1 parent e793631 commit d74c157

File tree

4 files changed

+89
-16
lines changed

4 files changed

+89
-16
lines changed

python/ray/dashboard/modules/state/state_head.py

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030
from ray.dashboard.subprocesses.module import SubprocessModule
3131
from ray.dashboard.subprocesses.routes import SubprocessRouteTable as routes
3232
from ray.dashboard.subprocesses.utils import ResponseType
33-
from ray.dashboard.utils import RateLimitedModule
33+
from ray.dashboard.utils import HTTPStatusCode, RateLimitedModule
3434
from ray.util.state.common import (
3535
DEFAULT_DOWNLOAD_FILENAME,
3636
DEFAULT_LOG_LIMIT,
@@ -86,7 +86,7 @@ def __init__(self, *args, **kwargs):
8686

8787
async def limit_handler_(self):
8888
return do_reply(
89-
success=False,
89+
status_code=HTTPStatusCode.TOO_MANY_REQUESTS,
9090
error_message=(
9191
"Max number of in-progress requests="
9292
f"{self.max_num_call_} reached. "
@@ -110,12 +110,16 @@ async def list_jobs(self, req: aiohttp.web.Request) -> aiohttp.web.Response:
110110
try:
111111
result = await self._state_api.list_jobs(option=options_from_req(req))
112112
return do_reply(
113-
success=True,
113+
status_code=HTTPStatusCode.OK,
114114
error_message="",
115115
result=asdict(result),
116116
)
117117
except DataSourceUnavailable as e:
118-
return do_reply(success=False, error_message=str(e), result=None)
118+
return do_reply(
119+
status_code=HTTPStatusCode.INTERNAL_ERROR,
120+
error_message=str(e),
121+
result=None,
122+
)
119123

120124
@routes.get("/api/v0/nodes")
121125
@RateLimitedModule.enforce_max_concurrent_calls
@@ -171,7 +175,7 @@ async def list_logs(self, req: aiohttp.web.Request) -> aiohttp.web.Response:
171175

172176
if not node_id and not node_ip:
173177
return do_reply(
174-
success=False,
178+
status_code=HTTPStatusCode.BAD_REQUEST,
175179
error_message=(
176180
"Both node id and node ip are not provided. "
177181
"Please provide at least one of them."
@@ -182,7 +186,7 @@ async def list_logs(self, req: aiohttp.web.Request) -> aiohttp.web.Response:
182186
node_id = await self._log_api.ip_to_node_id(node_ip)
183187
if not node_id:
184188
return do_reply(
185-
success=False,
189+
status_code=HTTPStatusCode.NOT_FOUND,
186190
error_message=(
187191
f"Cannot find matching node_id for a given node ip {node_ip}"
188192
),
@@ -195,12 +199,16 @@ async def list_logs(self, req: aiohttp.web.Request) -> aiohttp.web.Response:
195199
)
196200
except DataSourceUnavailable as e:
197201
return do_reply(
198-
success=False,
202+
status_code=HTTPStatusCode.INTERNAL_ERROR,
199203
error_message=str(e),
200204
result=None,
201205
)
202206

203-
return do_reply(success=True, error_message="", result=result)
207+
return do_reply(
208+
status_code=HTTPStatusCode.OK,
209+
error_message="",
210+
result=result,
211+
)
204212

205213
@routes.get("/api/v0/logs/{media_type}", resp_type=ResponseType.STREAM)
206214
@RateLimitedModule.enforce_max_concurrent_calls
@@ -330,7 +338,7 @@ async def delayed_response(self, req: aiohttp.web.Request):
330338
delay = int(req.match_info.get("delay_s", 10))
331339
await asyncio.sleep(delay)
332340
return do_reply(
333-
success=True,
341+
status_code=HTTPStatusCode.OK,
334342
error_message="",
335343
result={},
336344
partial_failure_warning=None,

python/ray/dashboard/state_api_utils.py

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,11 @@
2323
from ray.util.state.util import convert_string_to_type
2424

2525

26-
def do_reply(success: bool, error_message: str, result: ListApiResponse, **kwargs):
26+
def do_reply(
27+
status_code: HTTPStatusCode, error_message: str, result: ListApiResponse, **kwargs
28+
):
2729
return rest_response(
28-
status_code=HTTPStatusCode.OK if success else HTTPStatusCode.INTERNAL_ERROR,
30+
status_code=status_code,
2931
message=error_message,
3032
result=result,
3133
convert_google_style=False,
@@ -40,14 +42,22 @@ async def handle_list_api(
4042
try:
4143
result = await list_api_fn(option=options_from_req(req))
4244
return do_reply(
43-
success=True,
45+
status_code=HTTPStatusCode.OK,
4446
error_message="",
4547
result=asdict(result),
4648
)
4749
except ValueError as e:
48-
return do_reply(success=False, error_message=str(e), result=None)
50+
return do_reply(
51+
status_code=HTTPStatusCode.BAD_REQUEST,
52+
error_message=str(e),
53+
result=None,
54+
)
4955
except DataSourceUnavailable as e:
50-
return do_reply(success=False, error_message=str(e), result=None)
56+
return do_reply(
57+
status_code=HTTPStatusCode.INTERNAL_ERROR,
58+
error_message=str(e),
59+
result=None,
60+
)
5161

5262

5363
def _get_filters_from_req(
@@ -104,7 +114,7 @@ async def handle_summary_api(
104114
):
105115
result = await summary_fn(option=summary_options_from_req(req))
106116
return do_reply(
107-
success=True,
117+
status_code=HTTPStatusCode.OK,
108118
error_message="",
109119
result=asdict(result),
110120
)

python/ray/dashboard/utils.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,9 @@ class HTTPStatusCode(IntEnum):
4848
OK = 200
4949

5050
# 4xx Client Errors
51+
BAD_REQUEST = 400
5152
NOT_FOUND = 404
53+
TOO_MANY_REQUESTS = 429
5254

5355
# 5xx Server Errors
5456
INTERNAL_ERROR = 500

python/ray/tests/test_state_api.py

Lines changed: 54 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
import time
66
from collections import Counter
77
from concurrent.futures import ThreadPoolExecutor
8-
from typing import List
8+
from typing import List, Optional
99
from unittest.mock import AsyncMock, MagicMock, patch
1010

1111
import pytest
@@ -860,6 +860,59 @@ async def test_api_manager_list_workers(state_api_manager):
860860
assert exc_info.value.args[0] == GCS_QUERY_FAILURE_WARNING
861861

862862

863+
@pytest.mark.asyncio
864+
@pytest.mark.parametrize(
865+
("exception", "status_code"),
866+
[
867+
(None, 200),
868+
(ValueError("Invalid filter parameter"), 400),
869+
(DataSourceUnavailable("GCS connection failed"), 500),
870+
],
871+
)
872+
async def test_handle_list_api_status_codes(
873+
exception: Optional[Exception], status_code: int
874+
):
875+
"""Test that handle_list_api calls do_reply with correct status codes.
876+
877+
This directly tests the HTTP layer logic that maps exceptions to status codes:
878+
- Success → HTTP 200 OK
879+
- ValueError → HTTP 400 BAD_REQUEST
880+
- DataSourceUnavailable → HTTP 500 INTERNAL_ERROR
881+
"""
882+
from unittest.mock import AsyncMock, MagicMock
883+
884+
from ray.dashboard.state_api_utils import handle_list_api
885+
from ray.util.state.common import ListApiResponse
886+
887+
# 1. Mock aiohttp request with proper query interface
888+
mock_request = MagicMock()
889+
890+
def mock_get(key, default=None):
891+
return default
892+
893+
mock_request.query = MagicMock()
894+
mock_request.query.get = mock_get
895+
896+
# 2. Mock response whether success or failure.
897+
if exception is None:
898+
mock_backend = AsyncMock(
899+
return_value=ListApiResponse(
900+
result=[],
901+
total=0,
902+
num_after_truncation=0,
903+
num_filtered=0,
904+
partial_failure_warning="",
905+
)
906+
)
907+
else:
908+
mock_backend = AsyncMock(side_effect=exception)
909+
910+
response = await handle_list_api(mock_backend, mock_request)
911+
912+
# 3. Assert status_code is correct.
913+
assert response.status == status_code
914+
915+
863916
@pytest.mark.asyncio
864917
async def test_api_manager_list_tasks(state_api_manager):
865918
data_source_client = state_api_manager.data_source_client

0 commit comments

Comments
 (0)