mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-30 06:43:27 +00:00
fix(proxy): stop the reload worker pool on a broken edit instead of respawning it
Workers now load the app through a factory that exits with uvicorn's STARTUP_FAILURE code on any import error, so the pool stops and the reloader waits for the next save, the same as single-worker reload. Multi-worker reload needs uvicorn>=0.50.0 for that, so older versions keep the single-worker reload with a warning.
This commit is contained in:
parent
a9ce9454b4
commit
48729c7e73
4 changed files with 97 additions and 16 deletions
|
|
@ -39,7 +39,7 @@ Same applies for filing bug reports and feature requests, with .github/ISSUE_TEM
|
|||
|
||||
If you're resolving a linear ticket, in the "## Linear ticket" section of the PR, say "Resolves LIT-1234", replacing "LIT-1234" with the actual ticket id that you're resolving. If you don't have the ticket id, don't make one up or search for it. Just drop the section
|
||||
|
||||
Run every QA proxy with `--num_workers 2 --reload` and a dedicated Redis instance configured through `router_settings.redis_host`, `redis_port`, and credentials when required. Verify two application worker PIDs are serving, then edit a watched Python, config YAML, or `.env` file and verify both workers are replaced and requests still succeed. Use the same worker count and Redis topology for before and after proof
|
||||
Run every QA proxy with `--num_workers 2 --reload` and a dedicated Redis instance configured through `router_settings.redis_host`, `redis_port`, and credentials when required. `litellm/proxy/dev_config.yaml` sets no Redis, so QA from a copy of it that adds them. Verify two application worker PIDs are serving, then edit a watched Python, config YAML, or `.env` file and verify both workers are replaced and requests still succeed. Use the same worker count and Redis topology for before and after proof
|
||||
|
||||
Never use `pytest` commands or the like as "Screenshots / Proof of Fix". We prefer curl'ing a live proxy instance running on localhost:4000 (I like to run it with `python litellm/proxy/proxy_cli.py --config litellm/proxy/dev_config.yaml --num_workers 2 --detailed_debug --reload --use_v2_migration_resolver 2>&1 | tee litellm.log`. The Admin UI dev server is `npm run dev` in `ui/litellm-dashboard`, served on port 3000) and showing both the command run and the output. Also, it should hit real LLM provider APIs, not mocks, and cost real $$$ because that is the most realistic test. The proof of fix should be exactly what the end user / customer would see / do. The run logs in PR #27703 is a prime example of how to do it (not a huge fan of using a python test script that future me and the team will have no visibility into. I prefer just curl commands or a short list of bash commands (e.g., using `for`)). If it's a UI thing, or the main use case runs through a headful agentic coding tool like Claude Code or Codex, drive that surface yourself and embed your own before and after screenshots of it in the PR (the Admin UI page, or what the coding tool shows), next to an ordered list of the URLs to go to (e.g., http://localhost:4000/ui/?page=logs), where to click, and what fields to fill out so a reviewer can reproduce it
|
||||
|
||||
|
|
|
|||
|
|
@ -1,17 +1,19 @@
|
|||
from collections.abc import Sequence
|
||||
import logging
|
||||
import sys
|
||||
from collections.abc import Callable, Sequence
|
||||
from functools import partial
|
||||
from inspect import signature
|
||||
from socket import socket
|
||||
from typing import Final, Literal
|
||||
|
||||
from pydantic import ConfigDict, JsonValue, TypeAdapter, with_config
|
||||
from pydantic import ConfigDict, ImportString, JsonValue, TypeAdapter, with_config
|
||||
from typing_extensions import ReadOnly, Required, TypedDict
|
||||
from uvicorn import Config, Server
|
||||
from uvicorn import config as uvicorn_config
|
||||
from uvicorn.supervisors import ChangeReload, Multiprocess
|
||||
|
||||
|
||||
class WorkerOptions(TypedDict, total=False):
|
||||
app: Required[ReadOnly[str]]
|
||||
class ServerOptions(TypedDict, total=False):
|
||||
host: ReadOnly[str]
|
||||
port: ReadOnly[int]
|
||||
server_header: ReadOnly[bool]
|
||||
|
|
@ -28,14 +30,38 @@ class WorkerOptions(TypedDict, total=False):
|
|||
|
||||
|
||||
@with_config(ConfigDict(extra="forbid"))
|
||||
class ReloadOptions(WorkerOptions):
|
||||
class ReloadOptions(ServerOptions):
|
||||
app: Required[ReadOnly[str]]
|
||||
reload: ReadOnly[bool]
|
||||
reload_dirs: ReadOnly[tuple[str, ...]]
|
||||
reload_includes: ReadOnly[tuple[str, ...]]
|
||||
|
||||
|
||||
RELOAD_OPTIONS: Final = TypeAdapter(ReloadOptions)
|
||||
WORKER_OPTIONS: Final = TypeAdapter(WorkerOptions)
|
||||
SERVER_OPTIONS: Final = TypeAdapter(ServerOptions)
|
||||
ASGI_APP: Final = TypeAdapter(ImportString[Callable[..., object]])
|
||||
LOGGER: Final = logging.getLogger("uvicorn.error")
|
||||
|
||||
|
||||
def multi_worker_reload_supported() -> bool:
|
||||
return hasattr(uvicorn_config, "STARTUP_FAILURE")
|
||||
|
||||
|
||||
def load_app_or_exit(import_path: str) -> Callable[..., object]:
|
||||
try:
|
||||
return ASGI_APP.validate_python(import_path)
|
||||
except Exception:
|
||||
LOGGER.exception("Error loading ASGI app %s", import_path)
|
||||
sys.exit(uvicorn_config.STARTUP_FAILURE)
|
||||
|
||||
|
||||
def pool_config(options: ReloadOptions, workers: int) -> Config:
|
||||
return Config(
|
||||
partial(load_app_or_exit, options["app"]),
|
||||
factory=True,
|
||||
workers=workers,
|
||||
**SERVER_OPTIONS.validate_python(options),
|
||||
)
|
||||
|
||||
|
||||
def _run_worker_pool(config: Config, sockets: Sequence[socket] | None) -> None:
|
||||
|
|
@ -53,14 +79,14 @@ def _run_worker_pool(config: Config, sockets: Sequence[socket] | None) -> None:
|
|||
|
||||
|
||||
def run_multi_worker_reload(options: ReloadOptions, workers: int) -> None:
|
||||
worker_options: Final = WORKER_OPTIONS.validate_python(options)
|
||||
worker_config: Final = Config(**worker_options, workers=workers)
|
||||
worker_config: Final = pool_config(options, workers)
|
||||
reload_config: Final = Config(
|
||||
**worker_options,
|
||||
options["app"],
|
||||
workers=1,
|
||||
reload=options["reload"],
|
||||
reload_dirs=list(options["reload_dirs"]),
|
||||
reload_includes=list(options["reload_includes"]),
|
||||
**SERVER_OPTIONS.validate_python(options),
|
||||
)
|
||||
shared_socket: Final = worker_config.bind_socket()
|
||||
try:
|
||||
|
|
|
|||
|
|
@ -1033,7 +1033,7 @@ class ProxyInitializationHelpers:
|
|||
"--reload",
|
||||
is_flag=True,
|
||||
default=False,
|
||||
help="Enable uvicorn hot reload (dev only), including all --num_workers workers. Also reloads when the --config YAML or .env changes. Incompatible with --run_gunicorn, --run_hypercorn, and --run_granian.",
|
||||
help="Enable uvicorn hot reload (dev only), including every --num_workers worker on uvicorn>=0.50.0. Also reloads when the --config YAML or .env changes. Incompatible with --run_gunicorn, --run_hypercorn, and --run_granian.",
|
||||
)
|
||||
@click.option(
|
||||
"--prometheus_metrics_port",
|
||||
|
|
@ -1597,12 +1597,22 @@ def run_server(
|
|||
if num_workers > 1:
|
||||
start_query_engine_reaper()
|
||||
if reload and num_workers > 1:
|
||||
from litellm.proxy.dev_reload import RELOAD_OPTIONS, run_multi_worker_reload
|
||||
|
||||
run_multi_worker_reload(
|
||||
RELOAD_OPTIONS.validate_python(uvicorn_args), workers=TypeAdapter(int).validate_python(num_workers)
|
||||
from litellm.proxy.dev_reload import (
|
||||
RELOAD_OPTIONS,
|
||||
multi_worker_reload_supported,
|
||||
run_multi_worker_reload,
|
||||
)
|
||||
|
||||
if multi_worker_reload_supported():
|
||||
run_multi_worker_reload(
|
||||
RELOAD_OPTIONS.validate_python(uvicorn_args),
|
||||
workers=TypeAdapter(int).validate_python(num_workers),
|
||||
)
|
||||
return
|
||||
print(
|
||||
"\033[1;33mLiteLLM Proxy: --reload with --num_workers>1 requires uvicorn>=0.50.0, "
|
||||
"so only a single worker reloads.\033[0m"
|
||||
)
|
||||
return
|
||||
uvicorn.run(
|
||||
**uvicorn_args,
|
||||
workers=num_workers,
|
||||
|
|
|
|||
45
tests/unit/proxy/test_dev_reload.py
Normal file
45
tests/unit/proxy/test_dev_reload.py
Normal file
|
|
@ -0,0 +1,45 @@
|
|||
import importlib
|
||||
from pathlib import Path
|
||||
from typing import Final
|
||||
|
||||
import pytest
|
||||
from uvicorn import config as uvicorn_config
|
||||
|
||||
from litellm.proxy.dev_reload import ReloadOptions, load_app_or_exit, pool_config
|
||||
|
||||
UNCHANGED_LOGGING: Final = {"version": 1, "incremental": True}
|
||||
|
||||
|
||||
def _options(app: str) -> ReloadOptions:
|
||||
return {
|
||||
"app": app,
|
||||
"log_config": UNCHANGED_LOGGING,
|
||||
"reload": True,
|
||||
"reload_dirs": (),
|
||||
"reload_includes": ("*.py",),
|
||||
}
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
("module", "source"),
|
||||
(
|
||||
("reload_syntax_error_app", "def app(\n"),
|
||||
("reload_name_error_app", "app = undefined_name\n"),
|
||||
("reload_runtime_error_app", "raise RuntimeError('broken edit')\n"),
|
||||
("reload_missing_attribute_app", "value = 1\n"),
|
||||
),
|
||||
)
|
||||
def test_a_broken_app_exits_the_worker_as_a_startup_failure(
|
||||
tmp_path: Path, monkeypatch: pytest.MonkeyPatch, module: str, source: str
|
||||
) -> None:
|
||||
(tmp_path / f"{module}.py").write_text(source)
|
||||
monkeypatch.syspath_prepend(str(tmp_path))
|
||||
with pytest.raises(SystemExit) as exit_info:
|
||||
pool_config(_options(f"{module}:app"), workers=2).load()
|
||||
assert exit_info.value.code == uvicorn_config.STARTUP_FAILURE
|
||||
|
||||
|
||||
def test_a_working_app_loads_as_the_module_defines_it(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
(tmp_path / "reload_working_app.py").write_text("async def app(scope, receive, send):\n return None\n")
|
||||
monkeypatch.syspath_prepend(str(tmp_path))
|
||||
assert load_app_or_exit("reload_working_app:app") is importlib.import_module("reload_working_app").app
|
||||
Loading…
Add table
Reference in a new issue