-
Notifications
You must be signed in to change notification settings - Fork 6.9k
[Serve.llm] Remove ImageRetriever class and related tests from the LLM deployment module. #53980
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
… 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>
…M deployment module. (ray-project#53980) Signed-off-by: Kourosh Hakhamaneshi <kourosh@anyscale.com>
…M deployment module. (#53980) Signed-off-by: Kourosh Hakhamaneshi <kourosh@anyscale.com> Signed-off-by: elliot-barn <elliot.barnwell@anyscale.com>
NOTE: #53937 should be merged first.