Skip to content

Code refactoring in proxy#53644

Merged
edoakes merged 5 commits intomasterfrom
SERVE-824-abrar-handle_exception
Jun 10, 2025
Merged

Code refactoring in proxy#53644
edoakes merged 5 commits intomasterfrom
SERVE-824-abrar-handle_exception

Conversation

@abrarsheikh
Copy link
Copy Markdown
Contributor

No description provided.

Signed-off-by: abrar <abrar@anyscale.com>
Copilot AI review requested due to automatic review settings June 8, 2025 05:54
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

Refactors proxy error handling by extracting status mapping into helper functions and surfaces request timeout settings in tests.

  • Adds request_timeout_s to HTTP and gRPC options in tests for consistent timeout behavior.
  • Moves inline exception-to-status logic in proxy.py into get_http_response_status, send_http_response_on_exception, get_grpc_response_status, and set_grpc_code_and_details.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
python/ray/serve/tests/test_controller.py Added request_timeout_s to HTTP and gRPC options in tests.
python/ray/serve/tests/conftest.py Added request_timeout_s to default serve start options.
python/ray/serve/_private/proxy.py Replaced inline exception handlers with calls to new helpers.
python/ray/serve/_private/http_util.py Introduced get_http_response_status and send_http_response_on_exception.
python/ray/serve/_private/grpc_util.py Introduced get_grpc_response_status and set_grpc_code_and_details.
Comments suppressed due to low confidence (1)

python/ray/serve/_private/http_util.py:656

  • [nitpick] Add a docstring to get_http_response_status to explain its parameters and the mapping logic, improving clarity for future maintainers.
def get_http_response_status(

@abrarsheikh abrarsheikh added the go add ONLY when ready to merge, run all tests label Jun 8, 2025
@edoakes
Copy link
Copy Markdown
Collaborator

edoakes commented Jun 9, 2025

what's the context?

Signed-off-by: abrar <abrar@anyscale.com>
Copy link
Copy Markdown
Collaborator

@edoakes edoakes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, listen to the AI?

Signed-off-by: abrar <abrar@anyscale.com>
@edoakes
Copy link
Copy Markdown
Collaborator

edoakes commented Jun 9, 2025

merge conflict

@edoakes edoakes merged commit ab2f988 into master Jun 10, 2025
5 checks passed
@edoakes edoakes deleted the SERVE-824-abrar-handle_exception branch June 10, 2025 14:22
elliot-barn pushed a commit that referenced this pull request Jun 18, 2025
Signed-off-by: abrar <abrar@anyscale.com>
Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
elliot-barn pushed a commit that referenced this pull request Jul 2, 2025
Signed-off-by: abrar <abrar@anyscale.com>
Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

go add ONLY when ready to merge, run all tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants