From 727ab8dcc4b4bdbbbbf5f5baa9158ce4d912ccad Mon Sep 17 00:00:00 2001 From: Yuneng Jiang Date: Mon, 4 May 2026 20:05:24 -0700 Subject: [PATCH] [Fix] Proxy: Break managed-resources import cycle on Python 3.13 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Python 3.13 CCI smoke matrix surfaces a partially-initialized-module ImportError when loading the managed files hook chain: litellm.proxy.hooks/__init__ (mid-import) -> enterprise.enterprise_hooks -> litellm_enterprise.proxy.hooks.managed_files -> litellm.llms.base_llm.managed_resources.isolation -> litellm.proxy.management_endpoints.common_utils -> litellm.proxy.utils (re-enters litellm.proxy.hooks) The except ImportError block in hooks/__init__.py silently swallowed the failure, leaving managed_files unregistered and POST /files returning 500 "Managed files hook not found". Two-layer fix: - Inline the 3-line _user_has_admin_view check in isolation.py instead of importing it from litellm.proxy.management_endpoints.common_utils. litellm.llms.* should not depend on litellm.proxy.* — removing this layering violation breaks the cycle at its root. - Define PROXY_HOOKS and get_proxy_hook before the conditional enterprise import in litellm/proxy/hooks/__init__.py, so any future re-entry resolves the public names instead of hitting an ImportError on a partially-initialized module. Also fold in two unrelated CCI repairs surfaced in the same staging run: - tests/otel_tests/test_key_logging_callbacks.py: per-key gcs_bucket_name / gcs_path_service_account are now stripped by initialize_dynamic_callback_params, so the GCS client falls through to the env-only branch. Update the assertion to match the new "GCS_BUCKET_NAME is not set" message. - .circleci/config.yml: tests/pass_through_tests now resolves google-auth-library@10.x via the @google-cloud/vertexai 1.12.0 bump, which uses dynamic ESM imports Jest 29 cannot load without --experimental-vm-modules. Pass that flag in the Vertex JS test step. Adds tests/test_litellm/proxy/hooks/test_proxy_hooks_init.py as a regression guard: managed_files / managed_vector_stores must register, and isolation.py must not transitively import litellm.proxy.utils. --- .circleci/config.yml | 2 +- .../base_llm/managed_resources/isolation.py | 10 +++- litellm/proxy/hooks/__init__.py | 30 +++++++----- .../otel_tests/test_key_logging_callbacks.py | 2 +- .../proxy/hooks/test_proxy_hooks_init.py | 47 +++++++++++++++++++ 5 files changed, 74 insertions(+), 17 deletions(-) create mode 100644 tests/test_litellm/proxy/hooks/test_proxy_hooks_init.py diff --git a/.circleci/config.yml b/.circleci/config.yml index d2c4906ef6..a883c07021 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -1935,7 +1935,7 @@ jobs: name: Run Vertex AI, Google AI Studio Node.js tests command: | cd tests/pass_through_tests - npx jest . --verbose + NODE_OPTIONS=--experimental-vm-modules npx jest . --verbose no_output_timeout: 30m - run: name: Run tests diff --git a/litellm/llms/base_llm/managed_resources/isolation.py b/litellm/llms/base_llm/managed_resources/isolation.py index 4298d04462..340fd82242 100644 --- a/litellm/llms/base_llm/managed_resources/isolation.py +++ b/litellm/llms/base_llm/managed_resources/isolation.py @@ -11,8 +11,14 @@ unscoped query. from typing import Any, Dict, List, Optional -from litellm.proxy._types import UserAPIKeyAuth -from litellm.proxy.management_endpoints.common_utils import _user_has_admin_view +from litellm.proxy._types import LitellmUserRoles, UserAPIKeyAuth + + +def _user_has_admin_view(user_api_key_dict: UserAPIKeyAuth) -> bool: + return user_api_key_dict.user_role in ( + LitellmUserRoles.PROXY_ADMIN, + LitellmUserRoles.PROXY_ADMIN_VIEW_ONLY, + ) def build_list_page(items: List[Any], has_more: bool = False) -> Dict[str, Any]: diff --git a/litellm/proxy/hooks/__init__.py b/litellm/proxy/hooks/__init__.py index 790ebcd879..34505427d7 100644 --- a/litellm/proxy/hooks/__init__.py +++ b/litellm/proxy/hooks/__init__.py @@ -11,14 +11,10 @@ from .parallel_request_limiter import _PROXY_MaxParallelRequestsHandler from .parallel_request_limiter_v3 import _PROXY_MaxParallelRequestsHandler_v3 from .responses_id_security import ResponsesIDSecurity -### CHECK IF ENTERPRISE HOOKS ARE AVAILABLE #### - -try: - from enterprise.enterprise_hooks import ENTERPRISE_PROXY_HOOKS -except ImportError: - ENTERPRISE_PROXY_HOOKS = {} - -# List of all available hooks that can be enabled +# List of all available hooks that can be enabled. +# Defined before the enterprise import below so that any module re-imported +# transitively through `enterprise.enterprise_hooks` can resolve `PROXY_HOOKS` +# and `get_proxy_hook` from this partially-initialized module without circling. PROXY_HOOKS = { "max_budget_limiter": _PROXY_MaxBudgetLimiter, "parallel_request_limiter": _PROXY_MaxParallelRequestsHandler_v3, @@ -34,11 +30,6 @@ if os.getenv("LEGACY_MULTI_INSTANCE_RATE_LIMITING", "false").lower() == "true": PROXY_HOOKS["parallel_request_limiter"] = _PROXY_MaxParallelRequestsHandler -### update PROXY_HOOKS with ENTERPRISE_PROXY_HOOKS ### - -PROXY_HOOKS.update(ENTERPRISE_PROXY_HOOKS) - - def get_proxy_hook( hook_name: Union[ Literal[ @@ -58,3 +49,16 @@ def get_proxy_hook( f"Unknown hook: {hook_name}. Available hooks: {list(PROXY_HOOKS.keys())}" ) return PROXY_HOOKS[hook_name] + + +### CHECK IF ENTERPRISE HOOKS ARE AVAILABLE #### + +try: + from enterprise.enterprise_hooks import ENTERPRISE_PROXY_HOOKS +except ImportError: + ENTERPRISE_PROXY_HOOKS = {} + + +### update PROXY_HOOKS with ENTERPRISE_PROXY_HOOKS ### + +PROXY_HOOKS.update(ENTERPRISE_PROXY_HOOKS) diff --git a/tests/otel_tests/test_key_logging_callbacks.py b/tests/otel_tests/test_key_logging_callbacks.py index 96a13b8456..f3dbc10adb 100644 --- a/tests/otel_tests/test_key_logging_callbacks.py +++ b/tests/otel_tests/test_key_logging_callbacks.py @@ -64,6 +64,6 @@ async def test_key_logging_callbacks(): assert health_data["logging_callbacks"]["callbacks"] == ["gcs_bucket"] assert health_data["logging_callbacks"]["status"] == "unhealthy" assert ( - "Failed to load vertex credentials" + "GCS_BUCKET_NAME is not set in the environment" in health_data["logging_callbacks"]["details"] ) diff --git a/tests/test_litellm/proxy/hooks/test_proxy_hooks_init.py b/tests/test_litellm/proxy/hooks/test_proxy_hooks_init.py new file mode 100644 index 0000000000..23d8887a72 --- /dev/null +++ b/tests/test_litellm/proxy/hooks/test_proxy_hooks_init.py @@ -0,0 +1,47 @@ +"""Regression guard for the enterprise hook registration / import cycle. + +Python 3.13 is stricter about partially-initialized modules and surfaces +cycles that Python 3.12 silently tolerated. The previous bug: + + litellm.proxy.hooks.__init__ + -> enterprise.enterprise_hooks + -> litellm_enterprise.proxy.hooks.managed_files + -> litellm.llms.base_llm.managed_resources.isolation + -> litellm.proxy.management_endpoints.common_utils + -> litellm.proxy.utils (re-enters litellm.proxy.hooks mid-init) + +silently swallowed the ImportError in `hooks/__init__.py`, leaving +``managed_files`` unregistered and the /files endpoint returning 500. +""" + +from litellm.proxy.hooks import PROXY_HOOKS, get_proxy_hook + + +def test_managed_files_hook_registered(): + assert "managed_files" in PROXY_HOOKS + hook_cls = get_proxy_hook("managed_files") + assert hook_cls.__name__ == "_PROXY_LiteLLMManagedFiles" + + +def test_managed_vector_stores_hook_registered(): + assert "managed_vector_stores" in PROXY_HOOKS + hook_cls = get_proxy_hook("managed_vector_stores") + assert hook_cls.__name__ == "_PROXY_LiteLLMManagedVectorStores" + + +def test_isolation_module_does_not_pull_in_proxy_utils(): + """Layering guard: litellm.llms.* must not transitively import + litellm.proxy.utils, which would reintroduce the import cycle.""" + import importlib + import sys + + for mod in [ + "litellm.proxy.utils", + "litellm.proxy.management_endpoints.common_utils", + "litellm.llms.base_llm.managed_resources.isolation", + ]: + sys.modules.pop(mod, None) + + importlib.import_module("litellm.llms.base_llm.managed_resources.isolation") + assert "litellm.proxy.utils" not in sys.modules + assert "litellm.proxy.management_endpoints.common_utils" not in sys.modules