diff --git a/enterprise/litellm_enterprise/proxy/hooks/managed_files.py b/enterprise/litellm_enterprise/proxy/hooks/managed_files.py index 7a871e6e65d..8b22bd936a8 100644 --- a/enterprise/litellm_enterprise/proxy/hooks/managed_files.py +++ b/enterprise/litellm_enterprise/proxy/hooks/managed_files.py @@ -32,6 +32,8 @@ from litellm.integrations.custom_logger import CustomLogger from litellm.litellm_core_utils.prompt_templates.common_utils import ( extract_file_metadata, ) +from openai.types.file_deleted import FileDeleted + from litellm.llms.base_llm.files.transformation import BaseFileEndpoints from litellm.llms.base_llm.managed_resources.isolation import ( build_list_page, @@ -1765,7 +1767,7 @@ class _PROXY_LiteLLMManagedFiles(CustomLogger, BaseFileEndpoints): litellm_parent_otel_span: Optional[Span], llm_router: Router, **data: Dict, - ) -> OpenAIFileObject: + ) -> FileDeleted: # Check if file deletion should be blocked due to batch references await self._check_file_deletion_allowed(file_id) @@ -1773,7 +1775,6 @@ class _PROXY_LiteLLMManagedFiles(CustomLogger, BaseFileEndpoints): # file_id = convert_b64_uid_to_unified_uid(file_id) model_file_id_mapping = await self.get_model_file_id_mapping([file_id], litellm_parent_otel_span) - delete_response = None specific_model_file_id_mapping = model_file_id_mapping.get(file_id) if specific_model_file_id_mapping: # Remove conflicting keys from data to avoid duplicate keyword arguments @@ -1785,23 +1786,14 @@ class _PROXY_LiteLLMManagedFiles(CustomLogger, BaseFileEndpoints): if credentials is not None else filtered_data ) - delete_response = await llm_router.afile_delete(model=model_id, file_id=model_file_id, **router_kwargs) + await llm_router.afile_delete(model=model_id, file_id=model_file_id, **router_kwargs) - stored_file_object = await self.delete_unified_file_id(file_id, litellm_parent_otel_span) + await self.delete_unified_file_id(file_id, litellm_parent_otel_span) - # Record successful deletion metric only on actual success - if stored_file_object or delete_response: - prom_logger = self._get_prometheus_logger() - if prom_logger: - prom_logger.record_managed_file_deleted(result="success") - - if stored_file_object: - return stored_file_object - elif delete_response: - delete_response.id = file_id - return delete_response - else: - raise Exception(f"LiteLLM Managed File object with id={file_id} not found") + prom_logger = self._get_prometheus_logger() + if prom_logger: + prom_logger.record_managed_file_deleted(result="success") + return FileDeleted(id=file_id, object="file", deleted=True) async def afile_content( self, diff --git a/litellm/llms/base_llm/files/transformation.py b/litellm/llms/base_llm/files/transformation.py index 3f8fec354b7..6d16a1cea69 100644 --- a/litellm/llms/base_llm/files/transformation.py +++ b/litellm/llms/base_llm/files/transformation.py @@ -267,7 +267,7 @@ class BaseFileEndpoints(ABC): litellm_parent_otel_span: Span | None, llm_router: Router, **data: dict, - ) -> OpenAIFileObject: + ) -> FileDeleted: pass @abstractmethod diff --git a/litellm/llms/bedrock/files/transformation.py b/litellm/llms/bedrock/files/transformation.py index a7f1b380fe2..201911737b6 100644 --- a/litellm/llms/bedrock/files/transformation.py +++ b/litellm/llms/bedrock/files/transformation.py @@ -263,7 +263,7 @@ def _validate_file_id_against_configured_buckets( return validate_against(configured_bucket_names[-1]) -_REJECTED_FILE_ID_REQUEST_URL: Final = "https://docs.litellm.ai/docs" +_REJECTED_FILE_ID_REQUEST_URL: Final = "https://litellm.ai" def _rejected_file_id(reason: ValueError) -> BedrockError: @@ -301,26 +301,37 @@ _MANAGED_LISTING_PREFIX_BY_PURPOSE: Final = MappingProxyType( ) -def _managed_listing_prefix(configured_prefix: str, purpose: str | None) -> str: - managed_prefix: Final = ( - _MANAGED_LISTING_PREFIX_BY_PURPOSE.get(purpose, _ANY_MANAGED_LISTING_PREFIX) - if purpose - else _ANY_MANAGED_LISTING_PREFIX - ) +_EMPTY_LISTING_QUERY: Final = (("list-type", "2"), ("max-keys", "0")) + + +def _managed_listing_prefix(configured_prefix: str, purpose: str | None) -> str | None: + managed_prefix: Final = _MANAGED_LISTING_PREFIX_BY_PURPOSE.get(purpose) if purpose else _ANY_MANAGED_LISTING_PREFIX + if managed_prefix is None: + return None return f"{configured_prefix}/{managed_prefix}" if configured_prefix else managed_prefix +def _listing_query(configured_prefix: str, purpose: str | None) -> tuple[tuple[str, str], ...]: + listing_prefix: Final = _managed_listing_prefix(configured_prefix, purpose) + if listing_prefix is None: + return _EMPTY_LISTING_QUERY + return (("list-type", "2"), ("prefix", listing_prefix)) + + def _requested_listing_purpose(litellm_params: Mapping[str, object]) -> str | None: requested_purpose: Final = litellm_params.get(LIST_FILES_PURPOSE_PARAM) return requested_purpose if isinstance(requested_purpose, str) else None def _listing_bucket_name(litellm_params: Mapping[str, object], purpose: str | None) -> str: - input_bucket_name: Final = get_configured_s3_bucket_name(litellm_params) if purpose != "batch_output": - return input_bucket_name + return get_configured_s3_bucket_name(litellm_params) trusted: Final = _trusted_s3_model_credentials(litellm_params) - return trusted.s3_output_bucket_name or os.getenv("AWS_S3_OUTPUT_BUCKET_NAME") or input_bucket_name + return ( + trusted.s3_output_bucket_name + or os.getenv("AWS_S3_OUTPUT_BUCKET_NAME") + or get_configured_s3_bucket_name(litellm_params) + ) def _listed_object_created_at(entry: ET.Element) -> int: @@ -1372,7 +1383,7 @@ class BedrockFilesConfig(BaseAWSLLM, BaseFilesConfig): ) target: Final = self._s3_request_target(optional_params=optional_params, litellm_params=litellm_params) url: Final = f"{target.endpoint_url}/{bucket_name}/" - listing_query: Final = (("list-type", "2"), ("prefix", _managed_listing_prefix(configured_prefix, purpose))) + listing_query: Final = _listing_query(configured_prefix, purpose) continuation_query: Final = (("continuation-token", continuation_token),) if continuation_token else () query: Final[dict[str, str]] = dict( # mutable-ok: the base files contract returns the query as a dict listing_query + continuation_query diff --git a/litellm/llms/custom_httpx/llm_http_handler.py b/litellm/llms/custom_httpx/llm_http_handler.py index 146f1d28386..b29185fec2e 100644 --- a/litellm/llms/custom_httpx/llm_http_handler.py +++ b/litellm/llms/custom_httpx/llm_http_handler.py @@ -59,6 +59,7 @@ from litellm.llms.base_llm.image_edit.transformation import BaseImageEditConfig from litellm.llms.base_llm.image_generation.transformation import ( BaseImageGenerationConfig, ) +from litellm.llms.base_llm.managed_resources.isolation import build_list_page from litellm.llms.base_llm.ocr.transformation import BaseOCRConfig, OCRResponse from litellm.llms.base_llm.realtime.http_transformation import BaseRealtimeHTTPConfig from litellm.llms.base_llm.realtime.transformation import BaseRealtimeConfig @@ -120,6 +121,7 @@ from litellm.types.llms.openai import ( CreateBatchRequest, CreateFileRequest, FileContentRequest, + FileListPage, HttpxBinaryResponseContent, OpenAIFileObject, ResponseInputParam, @@ -4899,7 +4901,7 @@ class BaseLLMHTTPHandler: _is_async: bool = False, client: HTTPHandler | AsyncHTTPHandler | None = None, timeout: float | httpx.Timeout | None = None, - ) -> list[OpenAIFileObject] | Coroutine[object, object, list[OpenAIFileObject]]: + ) -> FileListPage | Coroutine[object, object, FileListPage]: """ List all files """ @@ -4954,9 +4956,10 @@ class BaseLLMHTTPHandler: files_per_page: Final = self._files_per_listing_page( response, provider_config, logging_obj, litellm_params, headers, sync_httpx_client, timeout ) - return [ # mutable-ok: the base files contract returns a list + listed_files: Final = [ # mutable-ok: build_list_page takes the list the files contract returns listed_file for page_files in files_per_page for listed_file in page_files ] + return FileListPage(**build_list_page(listed_files)) async def async_list_files( self, @@ -4967,7 +4970,7 @@ class BaseLLMHTTPHandler: logging_obj: LiteLLMLoggingObj, client: HTTPHandler | AsyncHTTPHandler | None = None, timeout: float | httpx.Timeout | None = None, - ) -> list[OpenAIFileObject]: + ) -> FileListPage: """ Async list all files """ @@ -5011,9 +5014,10 @@ class BaseLLMHTTPHandler: files_per_page: Final = self._files_per_async_listing_page( response, provider_config, logging_obj, litellm_params, headers, async_httpx_client, timeout ) - return [ # mutable-ok: the base files contract returns a list + listed_files: Final = [ # mutable-ok: build_list_page takes the list the files contract returns listed_file async for page_files in files_per_page for listed_file in page_files ] + return FileListPage(**build_list_page(listed_files)) def _files_per_listing_page( self, diff --git a/tests/test_litellm/enterprise/proxy/test_managed_files_hook.py b/tests/test_litellm/enterprise/proxy/test_managed_files_hook.py index 36359dd1110..da4853294ad 100644 --- a/tests/test_litellm/enterprise/proxy/test_managed_files_hook.py +++ b/tests/test_litellm/enterprise/proxy/test_managed_files_hook.py @@ -1721,4 +1721,5 @@ async def test_afile_delete_bedrock_unified_id_end_to_end(monkeypatch): assert route.called assert route.calls[0].request.headers["Authorization"].startswith("AWS4-HMAC-SHA256") assert response.id == unified_file_id + assert response.model_dump() == {"id": unified_file_id, "object": "file", "deleted": True} managed_files.delete_unified_file_id.assert_awaited_once_with(unified_file_id, None) diff --git a/tests/test_litellm/llms/bedrock/files/test_bedrock_files_transformation.py b/tests/test_litellm/llms/bedrock/files/test_bedrock_files_transformation.py index 12b9f63c99b..f3506dc4045 100644 --- a/tests/test_litellm/llms/bedrock/files/test_bedrock_files_transformation.py +++ b/tests/test_litellm/llms/bedrock/files/test_bedrock_files_transformation.py @@ -3024,13 +3024,20 @@ class TestBedrockFileListTransformation: return_value=httpx.Response(200, content=self.LISTING) ) - files = litellm.file_list(custom_llm_provider="bedrock", purpose="batch", **_bedrock_s3_params()) + page = litellm.file_list(custom_llm_provider="bedrock", purpose="batch", **_bedrock_s3_params()) assert route.called request = route.calls[0].request assert request.headers["Authorization"].startswith("AWS4-HMAC-SHA256") assert _sent_signature(request.headers) == _s3_signature_for("GET", str(request.url), request.headers) - assert [file.id for file in files] == list(self.BATCH_IDS) + assert [file.id for file in page.data] == list(self.BATCH_IDS) + assert (page.object, page.first_id, page.last_id, page.has_more) == ( + "list", + self.BATCH_IDS[0], + self.BATCH_IDS[-1], + False, + ) + assert page.model_dump()["object"] == "list" def test_file_list_uses_trusted_snapshot_bucket_without_env(self, monkeypatch): import httpx @@ -3051,7 +3058,7 @@ class TestBedrockFileListTransformation: ) assert route.called - assert [file.id for file in files] == [*self.BATCH_IDS, self.OUTPUT_ID] + assert [file.id for file in files.data] == [*self.BATCH_IDS, self.OUTPUT_ID] @pytest.mark.asyncio async def test_afile_list_end_to_end_sends_signed_listing(self, monkeypatch): @@ -3076,7 +3083,8 @@ class TestBedrockFileListTransformation: assert route.called request = route.calls[0].request assert _sent_signature(request.headers) == _s3_signature_for("GET", str(request.url), request.headers) - assert [file.id for file in files] == [self.OUTPUT_ID] + assert [file.id for file in files.data] == [self.OUTPUT_ID] + assert (files.object, files.first_id, files.last_id, files.has_more) == ("list", self.OUTPUT_ID, self.OUTPUT_ID, False) def test_transform_list_files_request_narrows_prefix_to_requested_purpose(self, monkeypatch): from litellm.llms.bedrock.files.transformation import BedrockFilesConfig @@ -3133,6 +3141,73 @@ class TestBedrockFileListTransformation: assert (url, params) == (self.OUTPUT_BUCKET_URL, self.OUTPUT_QUERY) + EMPTY_LISTING = b""" + + my-bucket + + 0 + 0 + false +""" + NO_KEYS_QUERY = {"list-type": "2", "max-keys": "0"} + + def test_transform_list_files_request_asks_for_no_keys_when_bedrock_never_stores_the_purpose(self, monkeypatch): + import httpx + + from litellm.llms.bedrock.files.transformation import BedrockFilesConfig + + monkeypatch.setenv("AWS_S3_BUCKET_NAME", "my-bucket") + litellm_params = _bedrock_s3_params() + config = BedrockFilesConfig() + + url, params = config.transform_list_files_request( + purpose="user_data", optional_params={}, litellm_params=litellm_params + ) + next_request = config.transform_list_files_next_request( + raw_response=httpx.Response(200, content=self.EMPTY_LISTING), + optional_params={}, + litellm_params=litellm_params, + ) + + assert (url, params) == (self.BUCKET_URL, self.NO_KEYS_QUERY) + assert next_request is None + + def test_file_list_never_walks_the_bucket_for_a_purpose_bedrock_never_stores(self, monkeypatch): + import httpx + import respx + + import litellm + + monkeypatch.setenv("AWS_S3_BUCKET_NAME", "my-bucket") + + with respx.mock: + route = respx.get(self.BUCKET_URL, params__contains=self.NO_KEYS_QUERY).mock( + return_value=httpx.Response(200, content=self.EMPTY_LISTING) + ) + + page = litellm.file_list(custom_llm_provider="bedrock", purpose="user_data", **_bedrock_s3_params()) + + assert route.call_count == 1 + assert "prefix" not in route.calls[0].request.url.params + assert (page.data, page.has_more) == ([], False) + + def test_transform_list_files_request_lists_the_output_bucket_without_an_input_bucket(self, monkeypatch): + from litellm.llms.bedrock.files.transformation import BedrockFilesConfig + + monkeypatch.delenv("AWS_S3_BUCKET_NAME", raising=False) + monkeypatch.delenv("AWS_S3_OUTPUT_BUCKET_NAME", raising=False) + litellm_params = _trusted_bucket_snapshot(s3_output_bucket_name="my-output-bucket") + + url, params = BedrockFilesConfig().transform_list_files_request( + purpose="batch_output", optional_params={}, litellm_params=litellm_params + ) + + assert (url, params) == (self.OUTPUT_BUCKET_URL, self.OUTPUT_QUERY) + with pytest.raises(ValueError, match="s3_bucket_name"): + BedrockFilesConfig().transform_list_files_request( + purpose="batch", optional_params={}, litellm_params=dict(litellm_params) + ) + def test_transform_list_files_response_accepts_output_bucket_objects(self, monkeypatch): import httpx @@ -3178,7 +3253,7 @@ class TestBedrockFileListTransformation: assert route.called request = route.calls[0].request assert _sent_signature(request.headers) == _s3_signature_for("GET", str(request.url), request.headers) - assert [file.id for file in files] == [self.OUTPUT_BUCKET_ID] + assert [file.id for file in files.data] == [self.OUTPUT_BUCKET_ID] def test_transform_list_files_next_request_signs_the_continuation_page(self, monkeypatch): import httpx @@ -3275,7 +3350,7 @@ class TestBedrockFileListTransformation: **_trusted_bucket_snapshot(s3_bucket_name="my-bucket"), ) - self._assert_paged_listing(first_page, last_page, files) + self._assert_paged_listing(first_page, last_page, files.data) @pytest.mark.asyncio async def test_afile_list_follows_continuation_tokens_across_pages(self, monkeypatch): @@ -3297,7 +3372,7 @@ class TestBedrockFileListTransformation: **_trusted_bucket_snapshot(s3_bucket_name="my-bucket"), ) - self._assert_paged_listing(first_page, last_page, files) + self._assert_paged_listing(first_page, last_page, files.data) OVERSIZED_PAGE_SIZE = 3000 OVERSIZED_PAGE_COUNT = 6 @@ -3354,7 +3429,7 @@ class TestBedrockFileListTransformation: **_trusted_bucket_snapshot(s3_bucket_name="my-bucket"), ) - self._assert_capped_listing(route, files) + self._assert_capped_listing(route, files.data) @pytest.mark.asyncio async def test_afile_list_stops_at_the_openai_listing_ceiling(self, monkeypatch): @@ -3375,7 +3450,7 @@ class TestBedrockFileListTransformation: **_trusted_bucket_snapshot(s3_bucket_name="my-bucket"), ) - self._assert_capped_listing(route, files) + self._assert_capped_listing(route, files.data) def test_file_list_end_to_end_surfaces_the_s3_error_body(self, monkeypatch): import httpx