From 3d8e882ee066e505c298c98fc3cd2cb063d8a0ce Mon Sep 17 00:00:00 2001 From: Ephrim Stanley Date: Tue, 3 Mar 2026 11:01:49 -0500 Subject: [PATCH] Managed batches - Address PR bot comments from #22464 --- litellm/files/main.py | 2 +- litellm/llms/vertex_ai/batches/handler.py | 5 +- .../llms/vertex_ai/files/transformation.py | 5 +- .../test_file_retrieve_provider_routing.py | 127 ++++++++++++++++++ .../test_vertex_ai_files_transformation.py | 48 ++++++- 5 files changed, 181 insertions(+), 6 deletions(-) create mode 100644 tests/test_litellm/llms/vertex_ai/files/test_file_retrieve_provider_routing.py diff --git a/litellm/files/main.py b/litellm/files/main.py index 66d3a97468d..f0a8112fbdf 100644 --- a/litellm/files/main.py +++ b/litellm/files/main.py @@ -336,7 +336,7 @@ async def afile_retrieve( @client def file_retrieve( file_id: str, - custom_llm_provider: Literal["openai", "azure", "hosted_vllm", "manus"] = "openai", + custom_llm_provider: Literal["openai", "azure", "gemini", "vertex_ai", "hosted_vllm", "manus"] = "openai", extra_headers: Optional[Dict[str, str]] = None, extra_body: Optional[Dict[str, str]] = None, **kwargs, diff --git a/litellm/llms/vertex_ai/batches/handler.py b/litellm/llms/vertex_ai/batches/handler.py index ba3b5fb7a2c..5f1fefca963 100644 --- a/litellm/llms/vertex_ai/batches/handler.py +++ b/litellm/llms/vertex_ai/batches/handler.py @@ -115,9 +115,10 @@ class VertexAIBatchPrediction(VertexLLM): data=json.dumps(vertex_batch_request), ) except httpx.HTTPStatusError as e: - error_body = e.response.text if hasattr(e, 'response') else "N/A" + error_body = e.response.text litellm.verbose_logger.error( - f"Vertex AI batch create failed: status={e.response.status_code}, body={error_body[:1000]}" + "Vertex AI batch create failed: status=%s, body=%s", + e.response.status_code, error_body[:1000], ) raise if response.status_code != 200: diff --git a/litellm/llms/vertex_ai/files/transformation.py b/litellm/llms/vertex_ai/files/transformation.py index f0493cd6be9..bf3ed5e6ac9 100644 --- a/litellm/llms/vertex_ai/files/transformation.py +++ b/litellm/llms/vertex_ai/files/transformation.py @@ -408,10 +408,11 @@ class VertexAIFilesConfig(VertexBase, BaseFilesConfig): file_id = "deleted" if hasattr(raw_response, "request") and raw_response.request: url = str(raw_response.request.url) - if "/o/" in url: + if "/b/" in url and "/o/" in url: import urllib.parse + bucket_part = url.split("/b/")[-1].split("/o/")[0] encoded_name = url.split("/o/")[-1].split("?")[0] - file_id = f"gs://{urllib.parse.unquote(encoded_name)}" + file_id = f"gs://{bucket_part}/{urllib.parse.unquote(encoded_name)}" return FileDeleted(id=file_id, deleted=True, object="file") def transform_list_files_request( diff --git a/tests/test_litellm/llms/vertex_ai/files/test_file_retrieve_provider_routing.py b/tests/test_litellm/llms/vertex_ai/files/test_file_retrieve_provider_routing.py new file mode 100644 index 00000000000..68d5e2035f7 --- /dev/null +++ b/tests/test_litellm/llms/vertex_ai/files/test_file_retrieve_provider_routing.py @@ -0,0 +1,127 @@ +""" +Tests for Fix 1: file_retrieve Literal type was missing 'vertex_ai' and 'gemini', +causing a type mismatch when afile_retrieve delegated to the sync function. +""" + +import pytest +from unittest.mock import MagicMock, patch + +from litellm.files.main import file_retrieve + + +class TestFileRetrieveProviderRouting: + """ + Verify that file_retrieve accepts 'vertex_ai' and 'gemini' providers and + routes them through ProviderConfigManager / base_llm_http_handler. + """ + + def _make_mock_file_object(self): + mock = MagicMock() + mock.model_dump.return_value = { + "id": "gs://my-bucket/file.jsonl", + "object": "file", + "bytes": 1024, + "created_at": 0, + "filename": "file.jsonl", + "purpose": "batch", + "status": "processed", + } + return mock + + def test_should_route_vertex_ai_through_provider_config(self): + """ + Regression: file_retrieve Literal type was missing 'vertex_ai', + so passing custom_llm_provider='vertex_ai' would fail type-checking + and potentially cause a routing failure at runtime. + """ + mock_file = self._make_mock_file_object() + + with patch( + "litellm.files.main.base_llm_http_handler.retrieve_file", + return_value=mock_file, + ) as mock_retrieve: + result = file_retrieve( + file_id="gs://my-bucket/file.jsonl", + custom_llm_provider="vertex_ai", + ) + + mock_retrieve.assert_called_once() + assert result is not None + + def test_should_route_gemini_through_provider_config(self): + """ + Regression: file_retrieve Literal type was also missing 'gemini'. + """ + mock_file = self._make_mock_file_object() + + with patch( + "litellm.files.main.base_llm_http_handler.retrieve_file", + return_value=mock_file, + ) as mock_retrieve: + result = file_retrieve( + file_id="some-gemini-file-id", + custom_llm_provider="gemini", + ) + + mock_retrieve.assert_called_once() + assert result is not None + + def test_should_pass_file_id_to_handler_for_vertex_ai(self): + """Verify the file_id is forwarded correctly to the underlying handler.""" + mock_file = self._make_mock_file_object() + expected_file_id = "gs://my-bucket/path/to/file.jsonl" + + with patch( + "litellm.files.main.base_llm_http_handler.retrieve_file", + return_value=mock_file, + ) as mock_retrieve: + file_retrieve( + file_id=expected_file_id, + custom_llm_provider="vertex_ai", + ) + + call_kwargs = mock_retrieve.call_args.kwargs + assert call_kwargs.get("file_id") == expected_file_id + + def test_should_not_raise_bad_request_for_vertex_ai(self): + """ + Before the fix, vertex_ai fell through to the else-branch which raised + BadRequestError. Verify it no longer does. + """ + import litellm + + mock_file = self._make_mock_file_object() + + with patch( + "litellm.files.main.base_llm_http_handler.retrieve_file", + return_value=mock_file, + ): + try: + file_retrieve( + file_id="gs://my-bucket/file.jsonl", + custom_llm_provider="vertex_ai", + ) + except litellm.exceptions.BadRequestError as e: + pytest.fail( + f"file_retrieve raised BadRequestError for vertex_ai: {e}" + ) + + def test_should_not_raise_bad_request_for_gemini(self): + """Same as above but for 'gemini'.""" + import litellm + + mock_file = self._make_mock_file_object() + + with patch( + "litellm.files.main.base_llm_http_handler.retrieve_file", + return_value=mock_file, + ): + try: + file_retrieve( + file_id="some-file-id", + custom_llm_provider="gemini", + ) + except litellm.exceptions.BadRequestError as e: + pytest.fail( + f"file_retrieve raised BadRequestError for gemini: {e}" + ) diff --git a/tests/test_litellm/llms/vertex_ai/files/test_vertex_ai_files_transformation.py b/tests/test_litellm/llms/vertex_ai/files/test_vertex_ai_files_transformation.py index 6f1d753484d..598ad255aca 100644 --- a/tests/test_litellm/llms/vertex_ai/files/test_vertex_ai_files_transformation.py +++ b/tests/test_litellm/llms/vertex_ai/files/test_vertex_ai_files_transformation.py @@ -167,7 +167,7 @@ class TestTransformDeleteFile: assert isinstance(result, FileDeleted) assert result.deleted is True assert result.object == "file" - assert "litellm-vertex-files/publishers/google/models/gemini-2.0-flash-001/abc" in result.id + assert result.id == "gs://my-bucket/litellm-vertex-files/publishers/google/models/gemini-2.0-flash-001/abc" def test_should_fallback_to_deleted_id_when_no_request(self, config): raw_response = MagicMock(spec=httpx.Response) @@ -182,3 +182,49 @@ class TestTransformDeleteFile: assert isinstance(result, FileDeleted) assert result.id == "deleted" assert result.deleted is True + + def test_should_include_bucket_name_in_reconstructed_delete_id(self, config): + """ + Regression: the old code split on /o/ only, dropping the bucket from + the reconstructed gs:// URI. e.g. gs://path/to/file instead of + gs://my-bucket/path/to/file. + """ + raw_response = MagicMock(spec=httpx.Response) + mock_request = MagicMock() + encoded_object = urllib.parse.quote("path/to/file.jsonl", safe="") + mock_request.url = ( + f"https://storage.googleapis.com/storage/v1/b/my-bucket/o/{encoded_object}" + ) + raw_response.request = mock_request + + result = config.transform_delete_file_response( + raw_response=raw_response, + logging_obj=MagicMock(), + litellm_params={}, + ) + + assert result.id == "gs://my-bucket/path/to/file.jsonl" + + def test_should_include_bucket_in_nested_object_path(self, config): + """Verify bucket extraction works with deeply nested GCS object paths.""" + raw_response = MagicMock(spec=httpx.Response) + mock_request = MagicMock() + encoded_object = urllib.parse.quote( + "litellm-vertex-files/publishers/google/models/gemini-2.0-flash-001/abc-123", + safe="", + ) + mock_request.url = ( + f"https://storage.googleapis.com/storage/v1/b/prod-bucket/o/{encoded_object}" + ) + raw_response.request = mock_request + + result = config.transform_delete_file_response( + raw_response=raw_response, + logging_obj=MagicMock(), + litellm_params={}, + ) + + assert result.id == ( + "gs://prod-bucket/litellm-vertex-files/publishers/google/" + "models/gemini-2.0-flash-001/abc-123" + )