8000 [Serve.llm] Remove ImageRetriever class and related tests from the LLM deployment module. by kouroshHakha · Pull Request #53980 · ray-project/ray · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

[Serve.llm] Remove ImageRetriever class and related tests from the LLM deployment module. #53980

New issue
Merged
merged 12 commits into from
Jun 23, 2025

Conversation

kouroshHakha
Copy link
Contributor
@kouroshHakha kouroshHakha commented Jun 20, 2025

NOTE: #53937 should be merged first.

Signed-off-by: Kourosh Hakhamaneshi <kourosh@anyscale.com>
Signed-off-by: Kourosh Hakhamaneshi <kourosh@anyscale.com>
Signed-off-by: Kourosh Hakhamaneshi <kourosh@anyscale.com>
Signed-off-by: Kourosh Hakhamaneshi <kourosh@anyscale.com>
Signed-off-by: Kourosh Hakhamaneshi <kourosh@anyscale.com>
Signed-off-by: Kourosh Hakhamaneshi <kourosh@anyscale.com>
Signed-off-by: Kourosh Hakhamaneshi <kourosh@anyscale.com>
… module.

Signed-off-by: Kourosh Hakhamaneshi <kourosh@anyscale.com>
Signed-off-by: Kourosh Hakhamaneshi <kourosh@anyscale.com>
@kouroshHakha kouroshHakha marked this pull request as ready for review June 20, 2025 21:45
@Copilot Copilot AI review requested due to automatic review settings June 20, 2025 21:45
@kouroshHakha kouroshHakha requested a review from a team as a code owner June 20, 2025 21:45
@kouroshHakha kouroshHakha added the go add ONLY when ready to merge, run all tests label Jun 20, 2025
Copy link
Contributor
@Copilot 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

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>
Signed-off-by: Kourosh Hakhamaneshi <kourosh@anyscale.com>
Signed-off-by: Kourosh Hakhamaneshi <kourosh@anyscale.com>
@kouroshHakha kouroshHakha merged commit c202bea into ray-project:master Jun 23, 2025
5 checks passed
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
Labels
go add ONLY when ready to merge, run all tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0