-
Notifications
You must be signed in to change notification settings - Fork 6.5k
[Serve.llm] Remove ImageRetriever class and related tests from the LLM deployment module. #53980
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… module. Signed-off-by: Kourosh Hakhamaneshi <kourosh@anyscale.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR removes the obsolete ImageRetriever class and its related tests from the LLM deployment module, while also making minor updates to integration test configurations and internal engine health checks.
- Removed tests and mocks related to ImageRetriever.
- Updated integration test naming and test script commands.
- Refactored internal engine initialization and health-check logging.
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
release/release_tests.yaml | Updated test configuration and integration test names/script command |
release/llm_tests/serve/test_llm_serve_integration.py | Added enforce_eager parameter for engine initialization |
release/llm_tests/serve/test_llm_serve_fault_tolerance.py | Introduced fault-tolerance test for replica recovery |
python/ray/llm/tests/serve/mocks/fake_image_retriever.py | Removed obsolete fake image retriever mock |
python/ray/llm/tests/serve/cpu/deployments/routers/test_router.py | Removed redundant assertion on health-check call counts |
python/ray/llm/tests/serve/cpu/deployments/llm/test_image_retriever.py | Removed tests for the deleted ImageRetriever class |
python/ray/llm/tests/serve/cpu/deployments/llm/multiplex/test_lora_deployment_base_client.py | Removed dependency on FakeImageRetriever |
python/ray/llm/_internal/serve/deployments/routers/router.py | Refactored health check logic by removing an explicit gather of remote calls |
python/ray/llm/_internal/serve/deployments/prefill_decode_disagg/prefill_decode_disagg.py | Removed the inline FakeEngine class and introduced a _default_engine_cls attribute |
python/ray/llm/_internal/serve/deployments/llm/vllm/vllm_engine.py | Updated health-check logging from exception to error logging |
python/ray/llm/_internal/serve/deployments/llm/llm_server.py | Removed image retriever dependency injection and updated engine class selection |
python/ray/llm/_internal/serve/deployments/llm/image_retriever.py | Removed the ImageRetriever class as part of the cleanup |
Comments suppressed due to low confidence (6)
release/release_tests.yaml:4310
- Ensure that the updated test script, which now runs two test files, is intentional and that all downstream configurations referencing these tests are updated accordingly.
script: pytest -vs test_llm_serve_integration.py test_llm_serve_fault_tolerance.py
python/ray/llm/_internal/serve/deployments/llm/llm_server.py:405
- Since the ImageRetriever dependency injection has been removed, please update the class docstring and any relevant documentation to reflect that image retrieval is no longer supported.
)
python/ray/llm/_internal/serve/deployments/prefill_decode_disagg/prefill_decode_disagg.py:83
- The removal of the FakeEngine inline class is appropriate for production; please ensure that corresponding tests cover engine initialization and that related comments are updated to avoid confusion.
prefill_server: DeploymentHandle,
python/ray/llm/_internal/serve/deployments/llm/vllm/vllm_engine.py:817
- Switching from logger.exception to logger.error for health check failures reduces stack trace logging; confirm that this change still provides sufficient debugging information when a health check fails.
try:
Signed-off-by: Kourosh Hakhamaneshi <kourosh@anyscale.com>
lk-chen
approved these changes
Jun 23, 2025
minerharry
pushed a commit
to minerharry/ray
that referenced
this pull request
Jun 27, 2025
…M deployment module. (ray-project#53980) Signed-off-by: Kourosh Hakhamaneshi <kourosh@anyscale.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
NOTE: #53937 should be merged first.