mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-07 02:59:05 +00:00
fix(e2e): address greptile and veria review on vendor strategy suite
Some checks failed
LiteLLM Rust / rustfmt, clippy, test (push) Has been cancelled
Some checks failed
LiteLLM Rust / rustfmt, clippy, test (push) Has been cancelled
Store search tool keys as os.environ refs and resolve them in SearchAPIRouter. Tighten validation helpers and assertions so 5xx/empty/unrelated failures no longer pass coverage cells
This commit is contained in:
parent
4ec82d2e0d
commit
b4898b4797
6 changed files with 79 additions and 29 deletions
|
|
@ -37,8 +37,14 @@ class SearchAPIRouter:
|
|||
Returns:
|
||||
Tuple of (api_key, api_base) from tool configuration
|
||||
"""
|
||||
from litellm.secret_managers.main import get_secret_str
|
||||
|
||||
resolved_api_key: Optional[str] = tool_litellm_params.get("api_key")
|
||||
resolved_api_base: Optional[str] = tool_litellm_params.get("api_base")
|
||||
if isinstance(resolved_api_key, str) and resolved_api_key.startswith("os.environ/"):
|
||||
resolved_api_key = get_secret_str(resolved_api_key)
|
||||
if isinstance(resolved_api_base, str) and resolved_api_base.startswith("os.environ/"):
|
||||
resolved_api_base = get_secret_str(resolved_api_base)
|
||||
|
||||
return resolved_api_key, resolved_api_base
|
||||
|
||||
|
|
|
|||
|
|
@ -141,7 +141,6 @@ class StreamingResponse(BaseModel):
|
|||
# quota) arrive as SSE error events inside an otherwise-successful response;
|
||||
# the consumed body is elided, so this is the only place they surface.
|
||||
stream_error: str | None = None
|
||||
stream_done: bool = False
|
||||
|
||||
@property
|
||||
def ok(self) -> bool:
|
||||
|
|
@ -242,11 +241,8 @@ def assert_client_error(result: StreamingResponse, context: str) -> None:
|
|||
|
||||
|
||||
def assert_error_or_server_known(result: StreamingResponse, context: str) -> None:
|
||||
"""Missing required fields may be 4xx or 5xx per known acceptable proxy behavior."""
|
||||
assert result.status_code in range(400, 600), (
|
||||
f"{context}: expected error status, got {result.status_code}: {result.body[:300]}"
|
||||
)
|
||||
assert result.status_code != 200
|
||||
"""Require a deliberate client error; 5xx crashes must not count as validation coverage."""
|
||||
assert_client_error(result, context)
|
||||
|
||||
|
||||
def assert_auth_denied(result: StreamingResponse, context: str) -> None:
|
||||
|
|
|
|||
|
|
@ -234,12 +234,6 @@ class ImagesResult(BaseModel):
|
|||
data: list[ImageItem] = []
|
||||
|
||||
|
||||
class ImageEditForm(BaseModel):
|
||||
model: str
|
||||
prompt: str
|
||||
n: int = 1
|
||||
|
||||
|
||||
class TranscriptionResult(BaseModel):
|
||||
text: str = ""
|
||||
|
||||
|
|
|
|||
|
|
@ -12,7 +12,7 @@ import pytest
|
|||
from pydantic import BaseModel
|
||||
|
||||
from e2e_config import unique_marker
|
||||
from e2e_http import NoBody, unwrap, assert_client_error, assert_error_or_server_known
|
||||
from e2e_http import NoBody, unwrap, assert_client_error
|
||||
from lifecycle import ResourceManager
|
||||
from proxy_client import ProxyClient
|
||||
|
||||
|
|
@ -57,10 +57,11 @@ class SearchResponse(BaseModel):
|
|||
|
||||
|
||||
def _search_credentials() -> tuple[str, str]:
|
||||
if os.environ.get("PERPLEXITY_API_KEY"):
|
||||
return "perplexity", os.environ["PERPLEXITY_API_KEY"]
|
||||
if os.environ.get("PERPLEXITY_API_KEY") or os.environ.get("PERPLEXITYAI_API_KEY"):
|
||||
env_name = "PERPLEXITY_API_KEY" if os.environ.get("PERPLEXITY_API_KEY") else "PERPLEXITYAI_API_KEY"
|
||||
return "perplexity", f"os.environ/{env_name}"
|
||||
if os.environ.get("TAVILY_API_KEY"):
|
||||
return "tavily", os.environ["TAVILY_API_KEY"]
|
||||
return "tavily", "os.environ/TAVILY_API_KEY"
|
||||
pytest.fail("set PERPLEXITY_API_KEY or TAVILY_API_KEY for /v1/search e2e coverage")
|
||||
|
||||
|
||||
|
|
@ -118,7 +119,10 @@ class TestSearch:
|
|||
)
|
||||
)
|
||||
assert result.object in (None, "search")
|
||||
assert isinstance(result.results, list), f"expected results array: {result}"
|
||||
assert result.results, f"search returned no results: {result}"
|
||||
assert any(item.title or item.url for item in result.results), (
|
||||
f"search results lack title/url: {result}"
|
||||
)
|
||||
|
||||
@pytest.mark.covers("llm.search.openai.basic.nonstream.works")
|
||||
@pytest.mark.parametrize("max_results", [1, 5, 10])
|
||||
|
|
@ -137,7 +141,10 @@ class TestSearch:
|
|||
response_type=SearchResponse,
|
||||
)
|
||||
)
|
||||
assert isinstance(result.results, list)
|
||||
assert result.results, f"search returned no results for max_results={max_results}"
|
||||
assert len(result.results) <= max_results, (
|
||||
f"expected <= {max_results} results, got {len(result.results)}"
|
||||
)
|
||||
|
||||
@pytest.mark.covers("llm.search.openai.input_validation.nonstream.works")
|
||||
def test_missing_query_returns_error(
|
||||
|
|
@ -150,7 +157,7 @@ class TestSearch:
|
|||
headers=proxy.transport.bearer(key),
|
||||
json=SearchRequest(search_tool_name=tool, max_results=3),
|
||||
)
|
||||
assert_error_or_server_known(result, "search missing query")
|
||||
assert_client_error(result, "search missing query")
|
||||
|
||||
@pytest.mark.covers("llm.search.openai.input_validation.nonstream.works")
|
||||
def test_empty_query_returns_client_error(
|
||||
|
|
|
|||
|
|
@ -13,7 +13,7 @@ import pytest
|
|||
from pydantic import BaseModel, ConfigDict
|
||||
|
||||
from e2e_config import POLL_INTERVAL, POLL_TIMEOUT, unique_marker
|
||||
from e2e_http import FileUploadForm, NoBody, unwrap, assert_error_or_server_known
|
||||
from e2e_http import FileUploadForm, NoBody, unwrap, assert_client_error
|
||||
from lifecycle import ResourceManager
|
||||
from models import LiteLLMParamsBody
|
||||
from proxy_client import ProxyClient
|
||||
|
|
@ -202,7 +202,7 @@ class TestVectorStores:
|
|||
headers=proxy.transport.bearer(key),
|
||||
json=VectorStoreSearchBody(max_num_results=10),
|
||||
)
|
||||
assert_error_or_server_known(result, "vector store search missing query")
|
||||
assert_client_error(result, "vector store search missing query")
|
||||
|
||||
@pytest.mark.covers("llm.vector_stores.openai.basic.nonstream.works")
|
||||
def test_file_attach_poll_and_search(
|
||||
|
|
@ -314,7 +314,7 @@ class TestVectorStores:
|
|||
headers=proxy.transport.bearer(key),
|
||||
json=VectorStoreSearchBody(query="", max_num_results=10),
|
||||
)
|
||||
assert result.status_code in (200, 400, 500), (
|
||||
assert result.status_code in (200, 400), (
|
||||
f"empty search query unexpected status {result.status_code}: {result.body[:300]}"
|
||||
)
|
||||
|
||||
|
|
@ -322,7 +322,7 @@ class TestVectorStores:
|
|||
def test_retrieve_invalid_id_returns_error(
|
||||
self, proxy: ProxyClient, resources: ResourceManager
|
||||
) -> None:
|
||||
from e2e_http import Success, UnauthorizedError, UnknownApiError
|
||||
from e2e_http import Success, UnknownApiError
|
||||
|
||||
key = _register_openai_model(proxy, resources)
|
||||
result = proxy.transport.get(
|
||||
|
|
@ -334,12 +334,16 @@ class TestVectorStores:
|
|||
match result:
|
||||
case Success():
|
||||
pytest.fail("invalid vector store id must not succeed")
|
||||
case UnknownApiError(status_code=status):
|
||||
assert status in (400, 401, 404, 500), f"unexpected status {status}"
|
||||
case UnauthorizedError():
|
||||
return
|
||||
case _:
|
||||
case UnknownApiError(status_code=status) if 400 <= status < 500:
|
||||
return
|
||||
case UnknownApiError(status_code=status, body=body):
|
||||
pytest.fail(
|
||||
f"invalid vector store id must be 4xx, got {status}: {body[:300]}"
|
||||
)
|
||||
case other:
|
||||
pytest.fail(
|
||||
f"invalid vector store id must be a client error, got {other!r}"
|
||||
)
|
||||
|
||||
@pytest.mark.covers("llm.vector_stores.openai.input_validation.nonstream.works")
|
||||
def test_invalid_chunking_returns_error(
|
||||
|
|
@ -365,4 +369,4 @@ class TestVectorStores:
|
|||
},
|
||||
),
|
||||
)
|
||||
assert_error_or_server_known(result, "invalid chunking strategy")
|
||||
assert_client_error(result, "invalid chunking strategy")
|
||||
|
|
|
|||
43
tests/test_litellm/router_utils/test_search_api_router.py
Normal file
43
tests/test_litellm/router_utils/test_search_api_router.py
Normal file
|
|
@ -0,0 +1,43 @@
|
|||
from litellm.router_utils.search_api_router import SearchAPIRouter
|
||||
|
||||
|
||||
def test_resolve_search_provider_credentials_expands_os_environ_refs(
|
||||
monkeypatch,
|
||||
) -> None:
|
||||
monkeypatch.setenv("E2E_SEARCH_API_KEY", "secret-from-env")
|
||||
monkeypatch.setenv("E2E_SEARCH_API_BASE", "https://search.example")
|
||||
|
||||
api_key, api_base = SearchAPIRouter._resolve_search_provider_credentials(
|
||||
tool_litellm_params={
|
||||
"api_key": "os.environ/E2E_SEARCH_API_KEY",
|
||||
"api_base": "os.environ/E2E_SEARCH_API_BASE",
|
||||
}
|
||||
)
|
||||
|
||||
assert api_key == "secret-from-env"
|
||||
assert api_base == "https://search.example"
|
||||
|
||||
|
||||
def test_resolve_search_provider_credentials_passes_through_plain_values() -> None:
|
||||
api_key, api_base = SearchAPIRouter._resolve_search_provider_credentials(
|
||||
tool_litellm_params={
|
||||
"api_key": "sk-literal",
|
||||
"api_base": "https://literal.example",
|
||||
}
|
||||
)
|
||||
|
||||
assert api_key == "sk-literal"
|
||||
assert api_base == "https://literal.example"
|
||||
|
||||
|
||||
def test_resolve_search_provider_credentials_missing_env_returns_none(
|
||||
monkeypatch,
|
||||
) -> None:
|
||||
monkeypatch.delenv("MISSING_SEARCH_KEY_XYZ", raising=False)
|
||||
|
||||
api_key, api_base = SearchAPIRouter._resolve_search_provider_credentials(
|
||||
tool_litellm_params={"api_key": "os.environ/MISSING_SEARCH_KEY_XYZ"}
|
||||
)
|
||||
|
||||
assert api_key is None
|
||||
assert api_base is None
|
||||
Loading…
Add table
Reference in a new issue