Skip to content

Commit 3d8aaa5

Browse files
authored
[serve] Fix serve agent 501 response (ray-project#39440)
TL;DR: the optimization added in ray-project#38836, which returns 501 if Serve dependencies aren't installed, doesn't work. Instead of returning a 501 response like it should, the serve agent returns a mysterious `TypeError: object Response can't be used in 'await' expression` The decorator `gracefully_handle_missing_serve_dependencies` doesn't return an async function, and the second decorator `init_ray_and_catch_exceptions` runs `await f`, `f` being the result of the first decorator. - Normally this works fine since `f` returns a coroutine (the original async function) - But when serve dependencies are not installed and `f` returns a 501 Response, await on the 501 Response fails. The tests `test_dashboard_does_not_depend_on_serve` and `test_agent_does_not_depend_on_serve` didn't catch this because it includes the assert statement inside a try-except block, so the assert statement was actually failing but it was being ignored by the except block 🤦 Fixed the test and also verified manually this time that the optimization works.
1 parent d842d77 commit 3d8aaa5

File tree

2 files changed

+6
-6
lines changed

2 files changed

+6
-6
lines changed

dashboard/modules/serve/serve_agent.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323

2424
def gracefully_handle_missing_serve_dependencies(func):
2525
@wraps(func)
26-
def check(self, *args, **kwargs):
26+
async def check(self, *args, **kwargs):
2727
try:
2828
from ray import serve # noqa: F401
2929
except ImportError:
@@ -34,7 +34,7 @@ def check(self, *args, **kwargs):
3434
'"ray[serve]"`.'
3535
),
3636
)
37-
return func(self, *args, **kwargs)
37+
return await func(self, *args, **kwargs)
3838

3939
return check
4040

dashboard/tests/test_dashboard.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -925,11 +925,11 @@ def test_dashboard_does_not_depend_on_serve():
925925
# Check that Serve-dependent features fail
926926
try:
927927
response = requests.get(f"http://{agent_url}/api/serve/deployments/")
928+
print(f"response status code: {response.status_code}, expected: 501")
928929
assert response.status_code == 501
929-
except Exception as e:
930+
except requests.ConnectionError as e:
930931
# Fail to connect to service is fine.
931932
print(e)
932-
assert True
933933

934934

935935
@pytest.mark.skipif(
@@ -963,11 +963,11 @@ def test_agent_does_not_depend_on_serve(shutdown_only):
963963
# Check that Serve-dependent features fail
964964
try:
965965
response = requests.get(f"http://{agent_url}/api/serve/deployments/")
966+
print(f"response status code: {response.status_code}, expected: 501")
966967
assert response.status_code == 501
967-
except Exception as e:
968+
except requests.ConnectionError as e:
968969
# Fail to connect to service is fine.
969970
print(e)
970-
assert True
971971

972972
# The agent should be dead if raylet exits.
973973
raylet_proc.kill()

0 commit comments

Comments
 (0)