Feat/support docs auth #36
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
WalkthroughThe PR introduces HTTP Basic authentication for API documentation endpoints. It adds configuration fields for docs credentials with UUID-based password generation, URL constants for documentation routes, authenticated endpoints serving Swagger UI and Redoc, improved error handling with timestamping, and comprehensive tests for application behavior. Changes
Sequence DiagramsequenceDiagram
participant Client
participant FastAPI as FastAPI Application
participant AuthValidator as HTTPBasic Validator
participant Settings as ServerConfig
participant Logger as framex.log
Client->>FastAPI: GET /docs
FastAPI->>AuthValidator: Check credentials
AuthValidator->>Settings: Retrieve docs_user<br/>& docs_password
rect rgb(200, 220, 255)
Note over AuthValidator: Credentials verification<br/>using secrets.compare_digest
alt Valid credentials
AuthValidator->>FastAPI: Auth success
FastAPI->>Client: 200 Swagger UI (HTML)
else Invalid/Missing credentials
AuthValidator->>Logger: Log failed attempt
Logger->>FastAPI: ✗
FastAPI->>Client: 401 Unauthorized
end
end
Client->>FastAPI: GET /api/v1/openapi.json
FastAPI->>AuthValidator: Check credentials
alt Valid credentials
AuthValidator->>FastAPI: Auth success
FastAPI->>Client: 200 OpenAPI schema (JSON)
else Invalid/Missing credentials
FastAPI->>Client: 401 Unauthorized
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
Note Unit test generation is an Early Access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
src/framex/consts.py (1)
4-6: Consider usingAPI_STRto constructOPENAPI_URLfor consistency.The
OPENAPI_URLhardcodes/api/v1instead of referencing the existingAPI_STRconstant. This could lead to inconsistency ifAPI_STRis updated in the future.🔎 Proposed fix
API_STR = "/api/v1" -OPENAPI_URL = "/api/v1/openapi.json" +OPENAPI_URL = f"{API_STR}/openapi.json" DOCS_URL = "/docs" REDOC_URL = "/redoc"src/framex/driver/application.py (1)
133-139: Consider addingREDOC_URLto excluded log paths for consistency.
DOCS_URLandOPENAPI_URLare excluded from response logging, butREDOC_URLis not. All documentation endpoints should likely receive the same treatment.🔎 Proposed fix
if ( not request.url.path.startswith(API_STR) - or request.url.path in [DOCS_URL, OPENAPI_URL, *settings.server.excluded_log_paths] + or request.url.path in [DOCS_URL, OPENAPI_URL, REDOC_URL, *settings.server.excluded_log_paths] or b"text/event-stream; charset=utf-8" in response.raw_headers[0] or response.headers.get("X-Raw-Output", "False") == "True" ):
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
pyproject.tomlsrc/framex/config.pysrc/framex/consts.pysrc/framex/driver/application.py
🧰 Additional context used
🪛 Ruff (0.14.10)
src/framex/driver/application.py
76-76: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
🔇 Additional comments (3)
src/framex/driver/application.py (2)
91-105: LGTM!The protected documentation endpoints are correctly implemented. Disabling FastAPI's default docs routes and replacing them with authenticated versions is a sound approach. Using
include_in_schema=Falseappropriately keeps these meta-endpoints hidden from the generated API spec.
107-118: LGTM!Propagating the exception's
headersto the JSON response is essential for HTTP Basic auth to work correctly—theWWW-Authenticateheader triggers the browser's login prompt on 401 responses.pyproject.toml (1)
17-17: No documented breaking changes for ray[serve] 2.53.0. Ray 2.53.0 release notes do not call out Serve breaking changes; documented updates focus on feature additions (in-place updates, async inference, autoscaling). The codebase uses standard Ray Serve APIs (DeploymentHandle, ray.init(), app.state.deployments_dict) which are unchanged. Pinned version is appropriate for reproducibility.
Comment on lines
+55
to
+63
| @model_validator(mode="after") | ||
| def validate_model(self) -> Self: | ||
| if self.docs_password == "": | ||
| key = str(uuid4()) | ||
| self.docs_password = key | ||
| from framex.log import logger | ||
|
|
||
| logger.warning(f"No docs_password set, generate a random key: {key}") | ||
| return self |
There was a problem hiding this comment.
Security risk: Logging generated password exposes credentials.
Logging the auto-generated docs_password at warning level will persist sensitive credentials in log files, potentially exposing them to unauthorized access. Credentials should never be logged.
🔎 Proposed fix
@model_validator(mode="after")
def validate_model(self) -> Self:
if self.docs_password == "":
key = str(uuid4())
self.docs_password = key
from framex.log import logger
- logger.warning(f"No docs_password set, generate a random key: {key}")
+ logger.warning("No docs_password set, a random password has been generated. Set 'docs_password' in configuration for persistent access.")
return self📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| @model_validator(mode="after") | |
| def validate_model(self) -> Self: | |
| if self.docs_password == "": | |
| key = str(uuid4()) | |
| self.docs_password = key | |
| from framex.log import logger | |
| logger.warning(f"No docs_password set, generate a random key: {key}") | |
| return self | |
| @model_validator(mode="after") | |
| def validate_model(self) -> Self: | |
| if self.docs_password == "": | |
| key = str(uuid4()) | |
| self.docs_password = key | |
| from framex.log import logger | |
| logger.warning("No docs_password set, a random password has been generated. Set 'docs_password' in configuration for persistent access.") | |
| return self |
🤖 Prompt for AI Agents
In src/framex/config.py around lines 55 to 63, the validator currently logs the
auto-generated docs_password (exposing credentials). Remove the sensitive value
from logs: do not include the generated key in the log message; instead log a
generic warning like "No docs_password set, generated a random key (not logged)"
or omit the log entirely. Ensure the import of logger remains if you keep a
generic message, and avoid printing or persisting the actual key anywhere in
logs or stdout; if the key must be surfaced to the operator, provide a secure
retrieval mechanism (e.g., write to a protected config store) rather than
logging it.
Comment on lines
76
to
89
| def authenticate(credentials: HTTPBasicCredentials = Depends(security)) -> str: | ||
| correct_username = secrets.compare_digest(credentials.username, settings.server.docs_user) | ||
| correct_password = secrets.compare_digest(credentials.password, settings.server.docs_password) | ||
| from framex.log import logger | ||
|
|
||
| logger.warning(f"authenticating {credentials.username} {credentials.password}") | ||
|
|
||
| if not (correct_username and correct_password): | ||
| raise HTTPException( | ||
| status_code=status.HTTP_401_UNAUTHORIZED, | ||
| detail="Incorrect user or password", | ||
| headers={"WWW-Authenticate": "Basic"}, | ||
| ) | ||
| return credentials.username |
There was a problem hiding this comment.
Critical security issue: Password logged in plaintext on every authentication attempt.
Line 81 logs both username and password on every authentication attempt. This exposes credentials in logs and is a serious security vulnerability.
🔎 Proposed fix - remove password logging entirely
def authenticate(credentials: HTTPBasicCredentials = Depends(security)) -> str:
correct_username = secrets.compare_digest(credentials.username, settings.server.docs_user)
correct_password = secrets.compare_digest(credentials.password, settings.server.docs_password)
- from framex.log import logger
-
- logger.warning(f"authenticating {credentials.username} {credentials.password}")
if not (correct_username and correct_password):
+ from framex.log import logger
+ logger.warning(f"Failed authentication attempt for user: {credentials.username}")
raise HTTPException(
status_code=status.HTTP_401_UNAUTHORIZED,
detail="Incorrect user or password",
headers={"WWW-Authenticate": "Basic"},
)
return credentials.username📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| def authenticate(credentials: HTTPBasicCredentials = Depends(security)) -> str: | |
| correct_username = secrets.compare_digest(credentials.username, settings.server.docs_user) | |
| correct_password = secrets.compare_digest(credentials.password, settings.server.docs_password) | |
| from framex.log import logger | |
| logger.warning(f"authenticating {credentials.username} {credentials.password}") | |
| if not (correct_username and correct_password): | |
| raise HTTPException( | |
| status_code=status.HTTP_401_UNAUTHORIZED, | |
| detail="Incorrect user or password", | |
| headers={"WWW-Authenticate": "Basic"}, | |
| ) | |
| return credentials.username | |
| def authenticate(credentials: HTTPBasicCredentials = Depends(security)) -> str: | |
| correct_username = secrets.compare_digest(credentials.username, settings.server.docs_user) | |
| correct_password = secrets.compare_digest(credentials.password, settings.server.docs_password) | |
| if not (correct_username and correct_password): | |
| from framex.log import logger | |
| logger.warning(f"Failed authentication attempt for user: {credentials.username}") | |
| raise HTTPException( | |
| status_code=status.HTTP_401_UNAUTHORIZED, | |
| detail="Incorrect user or password", | |
| headers={"WWW-Authenticate": "Basic"}, | |
| ) | |
| return credentials.username |
🧰 Tools
🪛 Ruff (0.14.10)
76-76: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
🤖 Prompt for AI Agents
In src/framex/driver/application.py around lines 76 to 89, the logger.warning
call currently emits both username and plaintext password; remove the password
from logs — replace the log with one that only records the username (or a masked
value) and use an appropriate level (e.g., logger.info or logger.debug) so
credentials are never written in plaintext; also scan nearby code to ensure no
other logs output credentials.
|
Here are the copyable unit test edits: Copyable Editsruff.toml@@ -1,93 +1,30 @@
-target-version = "py311"
-line-length = 119
-respect-gitignore = true
-exclude = [
- ".bzr",
- ".direnv",
- ".eggs",
- ".git",
- ".git-rewrite",
- ".hg",
- ".ipynb_checkpoints",
- ".mypy_cache",
- ".nox",
- ".pants.d",
- ".pyenv",
- ".pytest_cache",
- ".pytype",
- ".ruff_cache",
- ".svn",
- ".tox",
- ".venv",
- ".vscode",
- "__pypackages__",
- "_build",
- "buck-out",
- "build",
- "dist",
- "node_modules",
- "site-packages",
- ".venv",
- ".history",
-]
-
[lint]
-extend-select = [
- "A", # bad name
- "ANN", # flake8-annotations
- "ARG", # bad arguments
- "ASYNC", # async
- "B", # flake8-bugbear
- "C", # Complexity
- "C4", #flake8-comprehensions
- "E", # PEP8 errors
- "ERA", # commented-out-code
- "F", # PEP8 formatting
- "FLY", # static-join-to-f-string
- "FAST", # FastAPI (FAST)
- "FLY", # static-join-to-f-string
- "FURB", # refurb
- "I", # Import sorting
- "INP", # flake8-no-pep420
- "N", # name-conventions
- "PERF", # Perflint
- "PIE", # Improper comments
- "PTH", # use-pathlib
- "PT", # pytest
- "RET", # unnecessary-return
- "RUF", # specific rules
- "S", # flake8-bandit
- "SIM", # simplify
- "T10", # debug found
- "T201", # print found
- "TID", # tidy-imports
- "UP", # Pyupgrade upgrades
- "W", # PEP8 warnings
-]
+select = [
+ # Pyflakes - catches logical errors and unused imports
+ "F",
+ # flake8-bugbear - catches common bugs and design issues
+ "B",
+ # flake8-bandit - security vulnerabilities
+ "S",
+ # flake8-blind-except - catches bare except clauses
+ "BLE",
+ # flake8-debugger - catches debugger statements
+ "T10",
+ # flake8-executable - catches executable files
+ "EXE",
+ # flake8-unused-arguments - catches unused function arguments
+ "ARG",
+ # tryceratops - catches try/except anti-patterns
+ "TRY",
+ # Ruff-specific rules for bugs and errors
+ "RUF",
+ # pycodestyle subset rules - only error-level issues
+ "E7", # Syntax errors
+ "E9", # Runtime errors
-ignore = [
- "B008", # Do not perform function calls in argument defaults
- "B009", # get-attr-with-constant
- "B010", # set-attr-with-constant
- "ERA001",
- "C901",
- "E402", # Module level import not at top of file
- "ANN401", # Dynamically typed expressions (typing.Any) are disallowed in `**kwargs`
- "ANN204", # ANN204 Missing return type annotation for special method
- # "ANN201", # Missing return type annotation for public function
- "E501", # Line too long
]
-
+# suppress common non critical issues
+ignore = ["I001", "F401", "W291", "W293", "Q000", "ANN001", "ANN201", "UP006", "UP045", "E501", "S101", "EXE001"]
[lint.per-file-ignores]
-"tests/**/*.py" = ["S101", "ANN201", "ANN001", "ANN002"]
-
-[lint.flake8-builtins]
-builtins-ignorelist = ["input", "id", "bytes", "type"]
-
-[lint.isort]
-force-wrap-aliases = true
-combine-as-imports = true
-split-on-trailing-comma = true
-
-[lint.pydocstyle]
-convention = "numpy"
+"**/__init__.py" = ["E402"]
+"**/conftest.py" = ["E402"]tests/driver/init.pyThis is a new file. tests/driver/test_application.pyThis is a new file. tests/test_config.py@@ -1,42 +0,0 @@
-import pytest
-from pydantic_core import ValidationError
-
-from framex.config import AuthConfig
-
-
-def test_config():
- from framex.plugin import get_plugin_config
- from framex.plugins.proxy.config import ProxyPluginConfig
-
- cfg = get_plugin_config("proxy", ProxyPluginConfig)
- assert isinstance(cfg, ProxyPluginConfig)
- assert cfg.proxy_urls is not None
-
-
-def test_auth_config():
- AuthConfig(
- general_auth_keys=["abcdefg"],
- auth_urls=[
- "/api/v1/a/*",
- "/api/b/call",
- "/api/v1/c/*",
- ],
- special_auth_keys={"/api/v1/a/call": ["0123456789"], "/api/v1/c/*": ["0123456789a", "0123456789b"]},
- )
-
- AuthConfig(
- general_auth_keys=["abcdefg"],
- auth_urls=[
- "/api/v1/a/*",
- ],
- special_auth_keys={"/api/v1/a/call": ["0123456789"]},
- )
-
- with pytest.raises(ValidationError) as exc_info:
- AuthConfig(
- auth_urls=["/api/v1/*"],
- special_auth_keys={
- "/admin/login": ["0123456789"],
- },
- )
- assert "special_auth_keys url '/admin/login'" in str(exc_info.value)tests/test_consts.pyThis is a new file. |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/framex/driver/application.py (1)
165-170: CORS middleware is missingallow_originsparameter.The CORS configuration sets
allow_credentials=Truebut omitsallow_origins. Withoutallow_origins, cross-origin requests will be blocked. Note thatallow_origins=["*"]cannot be used withallow_credentials=Trueper CORS specification. If credentials are needed, specify explicit origins.🔎 Proposed fix (if credentials needed with specific origins)
application.add_middleware( CORSMiddleware, + allow_origins=["http://localhost:3000"], # Specify actual allowed origins allow_credentials=True, allow_methods=["*"], allow_headers=["*"], )🔎 Alternative fix (if credentials not needed)
application.add_middleware( CORSMiddleware, - allow_credentials=True, + allow_origins=["*"], + allow_credentials=False, allow_methods=["*"], allow_headers=["*"], )
♻️ Duplicate comments (1)
src/framex/config.py (1)
55-63: Security risk: Logging generated password exposes credentials.This issue was previously flagged. Logging the auto-generated
docs_passwordat warning level persists sensitive credentials in log files. Remove the password from the log message.🔎 Proposed fix
@model_validator(mode="after") def validate_model(self) -> Self: if self.docs_password == "": key = str(uuid4()) self.docs_password = key from framex.log import logger - logger.warning(f"No docs_password set, generate a random key: {key}") + logger.warning("No docs_password set, a random password has been generated. Set 'docs_password' in configuration for persistent access.") return self
🧹 Nitpick comments (3)
src/framex/consts.py (1)
4-6: Consider usingAPI_STRto constructOPENAPI_URL.
OPENAPI_URLhardcodes/api/v1instead of reusingAPI_STR, creating a maintenance burden if the API prefix changes.🔎 Proposed fix
-OPENAPI_URL = "/api/v1/openapi.json" +OPENAPI_URL = f"{API_STR}/openapi.json"tests/driver/test_application.py (1)
159-170: Lifespan tests have weak assertions.Both
test_lifespan_without_rayandtest_lifespan_with_raymake identical assertions (lifespan_context is not None), which doesn't validate differentiated behavior. Consider testing actual lifespan execution with a context manager or verifying thaton_startcallbacks are invoked whenuse_ray=False.src/framex/driver/application.py (1)
120-129: Consider sanitizing exception messages in production.The general exception handler returns
str(exc)directly in the response body. This may expose internal implementation details (stack traces, file paths, SQL queries) to clients. Consider returning a generic error message and logging the actual exception server-side.🔎 Proposed fix
@application.exception_handler(Exception) async def _general_exception_handler(request, exc): # noqa + from framex.log import logger + logger.exception(f"Unhandled exception: {exc}") return JSONResponse( status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, content={ "status": 500, - "message": str(exc), + "message": "Internal server error", "timestamp": pytz.timezone("Asia/Shanghai").localize(datetime.now()).strftime("%Y-%m-%d %H:%M:%S"), }, )
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
pyproject.tomlsrc/framex/config.pysrc/framex/consts.pysrc/framex/driver/application.pytests/driver/__init__.pytests/driver/test_application.py
🧰 Additional context used
🧬 Code graph analysis (2)
src/framex/config.py (1)
src/framex/cli.py (1)
framex(11-12)
tests/driver/test_application.py (1)
src/framex/driver/application.py (1)
create_fastapi_application(28-171)
🪛 Ruff (0.14.10)
src/framex/driver/application.py
76-76: Do not perform function call Depends in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
(B008)
tests/driver/test_application.py
103-103: Avoid specifying long messages outside the exception class
(TRY003)
116-116: Avoid specifying long messages outside the exception class
(TRY003)
🔇 Additional comments (5)
tests/driver/test_application.py (3)
15-34: LGTM! Well-structured basic configuration tests.Good coverage of default FastAPI configuration: disabled docs/openapi URLs, redirect_slashes disabled, and CORS middleware presence.
36-73: LGTM! Authentication tests properly validate the Basic auth flow.Good coverage of authentication scenarios including missing credentials, wrong credentials, and valid credentials for both docs and OpenAPI endpoints.
75-124: LGTM! Good exception handler test coverage.Tests properly verify HTTPException propagation, general exception handling with 500 status, and unicode message support. The static analysis warnings (TRY003) are false positives for test fixtures.
src/framex/driver/application.py (2)
74-89: Authentication implementation looks correct.Good use of
secrets.compare_digestfor timing-safe credential comparison. The previous password logging issue has been addressed - now only the username is logged on failed attempts.
91-105: LGTM! Well-implemented authenticated documentation endpoints.Clean implementation using FastAPI's built-in
get_swagger_ui_html,get_redoc_html, andget_openapiutilities. Properly excluded from schema to avoid circular references.
| "pydantic-settings>=2.10.1", | ||
| "pytz>=2025.2", | ||
| "ray[serve]==2.47.1", | ||
| "ray[serve]==2.53.0", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Ray 2.53.0 breaking changes from 2.47 release notes
💡 Result:
Do you want a concise list of breaking changes introduced in Ray 2.53.0 (vs. what was documented in 2.47), or the full release-note diff/links so you can review details?
🏁 Script executed:
# Find and examine application.py to understand Ray Serve usage
find . -name "application.py" -type fRepository: touale/FrameX-kit
Length of output: 95
🏁 Script executed:
# Also check the current pyproject.toml to see the context
cat pyproject.toml | head -30Repository: touale/FrameX-kit
Length of output: 735
🌐 Web query:
Ray 2.53.0 vs 2.47 breaking changes list
💡 Result:
Short answer — there is no single “breaking changes” document that lists only breaking changes for 2.47→2.53. Major user-impact changes / regressions reported between Ray 2.47 and the 2.53 timeframe are:
- Default uv run integration enabled (since ~2.47), which can change runtime_env / pip behavior and broke some existing environments. Reported as a breaking/regression issue. (github.com)
- RLlib / Tune checkpoint-metrics regression in 2.47.0 (checkpointed metrics/step counters not restored). (github.com)
- Ray Core / security and packaging changes across 2.51–2.52 (e.g., token auth added, Python 3.9 wheel EOL) that can affect upgrades and deployments. See release highlights. (github.com)
For the authoritative, per-release notes (2.47 → 2.48 → … → 2.53) and the full list of changes/compatibility notes, review the Ray GitHub releases page and the individual GitHub issues linked from those releases: https://github.com/ray-project/ray/releases. (github.com)
If you want, I can:
- extract and summarize all documented breaking/behavior-changing notes from each release 2.47.0 → 2.53.0, or
- produce a migration checklist of things to test before upgrading (runtime_env/uv, Tune/RLlib checkpointing, Serve autoscaling/deploy defaults, Python wheel compatibility). Which would you prefer?
Citations:
- 1: [core] default uv integration breaks runtime env pip packages (when running in existing environments?) ray-project/ray#54344
- 2: [RLlib] Checkpoint metrics loading with Tune is broken in 2.47.0 ray-project/ray#53877
- 3: https://github.com/ray-project/ray/releases?utm_source=openai
- 4: https://github.com/ray-project/ray/releases?utm_source=openai
🏁 Script executed:
# Examine application.py to see Ray Serve usage patterns
cat -n ./src/framex/driver/application.pyRepository: touale/FrameX-kit
Length of output: 7528
🏁 Script executed:
# Also check if there are any tests or documentation about Ray usage
find . -type f \( -name "*.py" -o -name "*.md" \) | xargs rg -l "ray\[serve\]|Ray|ray\.serve" | head -20Repository: touale/FrameX-kit
Length of output: 636
🏁 Script executed:
# Check ray_adapter.py for Ray Serve deployment patterns
cat -n ./src/framex/adapter/ray_adapter.pyRepository: touale/FrameX-kit
Length of output: 2196
🏁 Script executed:
# Search for any Ray Serve configuration or deployment setup
rg -B2 -A5 "ray\.serve|serve\.|DeploymentHandle|deployment" --type pyRepository: touale/FrameX-kit
Length of output: 29637
🌐 Web query:
Ray Serve 2.53.0 API changes breaking deployment handle calls
💡 Result:
Short answer: Ray Serve 2.53.0 tightened the DeploymentHandle/DeploymentResponse composition rules — passing a DeploymentResponse (or a response generator) implicitly into downstream handle calls from nested objects is no longer supported. Instead you must convert the response to an underlying Ray ObjectRef (or await it) before passing it downstream. See the docs and recommended fixes below.
What changed
- The Serve docs warn that passing a DeploymentResponse into downstream handle calls in nested objects is fully deprecated / not supported; streaming DeploymentResponseGenerator values also cannot be passed. [1][2]
How to fix your code
- If you currently do something like: downstream_handle.method.remote(upstream_handle.method.remote(...))
then either:- await the DeploymentResponse inside the deployment (async) before passing the value; or
- convert to an ObjectRef and pass that: await response._to_object_ref() (async) or response._to_object_ref_sync() (sync), then pass the ObjectRef to downstream calls. [1][2]
Relevant docs
- Model composition / deprecation note (passing DeploymentResponse in nested objects fully deprecated). [1]
- DeploymentHandle / DeploymentResponse and streaming response behavior (cannot pass DeploymentResponseGenerator, use await or convert to ObjectRef). [2]
If you want, paste a short snippet of the failing code and I’ll rewrite it to the supported pattern.
Sources:
[1] Ray Serve — Deploy Compositions of Models (warning about passing DeploymentResponse in nested objects).
[2] Ray Serve — DeploymentHandle / DeploymentResponse / DeploymentResponseGenerator docs.
Test Ray 2.53.0 for breaking changes in Serve deployments and runtime behavior.
This upgrade spans 6 minor versions and includes documented breaking changes: DeploymentResponse composition rules now prohibit passing responses into downstream handle calls without explicit await or ObjectRef conversion; default uv run integration enabled (affecting runtime_env/pip); Python 3.9 wheels EOL; and security/packaging changes. Verify streaming patterns in ray_adapter.py (.options(stream=True).remote()) and deployment initialization in __init__.py (serve.start() / serve.run()) are unaffected, and test the lifespan context behavior in application.py.
🤖 Prompt for AI Agents
In pyproject.toml around line 17, the Ray upgrade to "ray[serve]==2.53.0" may
introduce breaking changes; update the code and tests to validate and fix
Serve-related adjustments: in ray_adapter.py ensure all
.options(stream=True).remote() streaming calls remain compatible and adjust to
any API changes; in modules that pass DeploymentResponse objects into downstream
handle calls (search repo for .remote(handle) usages), make those calls await
the response or convert to ObjectRef before passing; in package entrypoints
(__init__.py) confirm serve.start() / serve.run() usage matches 2.53.0 semantics
and modify initialization accordingly; in application.py add tests or
adjustments to verify lifespan context behavior and ensure runtime_env/pip
interactions are correct under the new default uv loop integration. Run full
test suite and update code paths or dependency pinning based on failures.
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.
Summary by CodeRabbit
New Features
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.