mirror of
https://github.com/BerriAI/litellm.git
synced 2026-11-13 08:52:02 +00:00
fix(proxy): register a lazy feature whose module a sibling already imported
generate_snapshot skipped registration when the feature's module was already in sys.modules, treating "imported" as "registered". Importing one feature can drag a sibling in as a side effect: loading vector_stores imports its neighbour vector_store_management. That module was therefore never registered, contributed no routes, and the fragment was dropped from the snapshot entirely, taking POST /vector_store/new, /info, /update and /delete out of the OpenAPI spec. The Terraform provider audit caught them as "not served by the proxy". Gate on the set of modules loaded before registration began instead of live sys.modules, so a sibling import during the loop no longer masks a feature. This is the invariant the runtime loader already keeps explicitly in app.state.lazy_loaded. A route-prefix check would not work here: policy_engine and guardrails both own /policies/usage, so the first to register would mask the second. The snapshot gains back vector_store_management (33 fragments, not 32).
This commit is contained in:
parent
4b2b855da5
commit
509240885b
4 changed files with 3140 additions and 34 deletions
File diff suppressed because it is too large
Load diff
|
|
@ -85,11 +85,19 @@ def _normalize_operation_ids(paths: Dict[str, Dict]) -> None:
|
|||
break
|
||||
|
||||
|
||||
def _register_lazy_feature(app, feat) -> tuple[str, str] | None:
|
||||
"""Import and register one lazy feature. Returns (name, error) if it could not be loaded."""
|
||||
def _register_lazy_feature(app, feat, preloaded: frozenset) -> tuple[str, str] | None:
|
||||
"""Import and register one lazy feature. Returns (name, error) if it could not be loaded.
|
||||
|
||||
`preloaded` is the set of module paths imported before registration began, not live
|
||||
sys.modules. Importing one feature can drag a sibling in as a side effect (loading
|
||||
vector_stores imports vector_store_management), and an imported-but-unregistered
|
||||
module contributes no routes, so its fragment would silently vanish from the
|
||||
snapshot. The runtime loader has the same invariant and tracks it explicitly in
|
||||
app.state.lazy_loaded.
|
||||
"""
|
||||
import importlib
|
||||
|
||||
if feat.module_path in sys.modules:
|
||||
if feat.module_path in preloaded:
|
||||
return None
|
||||
try:
|
||||
feat.register_fn(app, importlib.import_module(feat.module_path))
|
||||
|
|
@ -110,8 +118,11 @@ def generate_snapshot(*, strict: bool = False) -> dict[str, dict]:
|
|||
from litellm.proxy._lazy_features import LAZY_FEATURES
|
||||
from litellm.proxy.proxy_server import app, ensure_unique_openapi_operation_ids
|
||||
|
||||
preloaded = frozenset(sys.modules)
|
||||
skipped = tuple(
|
||||
result for result in (_register_lazy_feature(app, feat) for feat in LAZY_FEATURES) if result is not None
|
||||
result
|
||||
for result in (_register_lazy_feature(app, feat, preloaded) for feat in LAZY_FEATURES)
|
||||
if result is not None
|
||||
)
|
||||
for name, error in skipped:
|
||||
sys.stderr.write(f"warning: skip {name}: {error}\n")
|
||||
|
|
|
|||
|
|
@ -127,19 +127,34 @@ class TestDriftedFeatures:
|
|||
|
||||
|
||||
class TestRegisterLazyFeature:
|
||||
def test_skips_a_module_that_is_already_imported(self):
|
||||
def test_skips_a_feature_that_was_already_loaded_before_registration_began(self):
|
||||
feature = SimpleNamespace(name="ok", module_path="json", register_fn=lambda app, module: None)
|
||||
|
||||
assert _register_lazy_feature(SimpleNamespace(), feature) is None
|
||||
assert _register_lazy_feature(SimpleNamespace(), feature, frozenset({"json"})) is None
|
||||
|
||||
def test_registers_a_module_that_imports_cleanly(self, monkeypatch):
|
||||
def test_registers_a_module_that_imports_cleanly(self):
|
||||
registered = []
|
||||
feature = SimpleNamespace(
|
||||
name="ok", module_path="json", register_fn=lambda app, module: registered.append(module.__name__)
|
||||
)
|
||||
monkeypatch.delitem(sys.modules, "json", raising=False)
|
||||
|
||||
assert _register_lazy_feature(SimpleNamespace(), feature) is None
|
||||
assert _register_lazy_feature(SimpleNamespace(), feature, frozenset()) is None
|
||||
assert registered == ["json"]
|
||||
|
||||
def test_registers_a_module_a_sibling_feature_dragged_into_sys_modules(self):
|
||||
"""Regression: importing vector_stores imports vector_store_management as a side effect.
|
||||
|
||||
Gating on live sys.modules treated that as "already registered", so its router was
|
||||
never included, it contributed no routes, and generate_snapshot dropped its whole
|
||||
fragment; POST /vector_store/new and friends disappeared from the OpenAPI spec.
|
||||
"""
|
||||
registered = []
|
||||
feature = SimpleNamespace(
|
||||
name="sibling", module_path="json", register_fn=lambda app, module: registered.append(module.__name__)
|
||||
)
|
||||
|
||||
assert "json" in sys.modules # imported, but nobody registered its routes
|
||||
assert _register_lazy_feature(SimpleNamespace(), feature, preloaded=frozenset()) is None
|
||||
assert registered == ["json"]
|
||||
|
||||
def test_reports_the_feature_and_error_when_the_module_is_missing(self):
|
||||
|
|
@ -147,21 +162,23 @@ class TestRegisterLazyFeature:
|
|||
name="broken", module_path="litellm_no_such_module", register_fn=lambda app, module: None
|
||||
)
|
||||
|
||||
result = _register_lazy_feature(SimpleNamespace(), feature)
|
||||
result = _register_lazy_feature(SimpleNamespace(), feature, frozenset())
|
||||
|
||||
assert result is not None
|
||||
name, error = result
|
||||
assert name == "broken"
|
||||
assert "ModuleNotFoundError" in error
|
||||
|
||||
def test_reports_the_error_when_registration_raises(self, monkeypatch):
|
||||
def test_reports_the_error_when_registration_raises(self):
|
||||
def _raise(app, module):
|
||||
raise ValueError("router blew up")
|
||||
|
||||
feature = SimpleNamespace(name="broken", module_path="json", register_fn=_raise)
|
||||
monkeypatch.delitem(sys.modules, "json", raising=False)
|
||||
|
||||
assert _register_lazy_feature(SimpleNamespace(), feature) == ("broken", "ValueError: router blew up")
|
||||
assert _register_lazy_feature(SimpleNamespace(), feature, frozenset()) == (
|
||||
"broken",
|
||||
"ValueError: router blew up",
|
||||
)
|
||||
|
||||
|
||||
class TestCheckSnapshot:
|
||||
|
|
|
|||
1165
ui/litellm-dashboard/src/lib/http/schema.d.ts
generated
vendored
1165
ui/litellm-dashboard/src/lib/http/schema.d.ts
generated
vendored
File diff suppressed because it is too large
Load diff
Loading…
Add table
Reference in a new issue