fix(s3_v2): fix test patch target and prevent double-encoding

- Fix test: use CustomBatchLogger.periodic_flush (not S3Logger._periodic_flush)
- Prevent double-encoding of pre-encoded keys by applying unquote()
  before quote(), making encoding idempotent (Greptile P2 feedback)
This commit is contained in:
yryzhan 2026-05-20 18:06:50 +02:00
parent 5b628b7bae
commit 41e054c798
2 changed files with 5 additions and 5 deletions

View file

@ -10,7 +10,7 @@ import asyncio
import time
from datetime import datetime
from typing import List, Optional, cast
from urllib.parse import quote
from urllib.parse import quote, unquote
import litellm
from litellm._logging import print_verbose, verbose_logger
@ -350,7 +350,7 @@ class S3Logger(CustomBatchLogger, BaseAWSLLM):
verbose_logger.debug(f"s3_v2 logger - s3_verify setting: {self.s3_verify}")
# Prepare the URL with percent-encoded object key
encoded_key = quote(batch_logging_element.s3_object_key, safe="/")
encoded_key = quote(unquote(batch_logging_element.s3_object_key), safe="/")
url = f"https://{self.s3_bucket_name}.s3.{self.s3_region_name}.amazonaws.com/{encoded_key}"
if self.s3_endpoint_url and self.s3_bucket_name:
@ -536,7 +536,7 @@ class S3Logger(CustomBatchLogger, BaseAWSLLM):
)
# Prepare the URL with percent-encoded object key
encoded_key = quote(batch_logging_element.s3_object_key, safe="/")
encoded_key = quote(unquote(batch_logging_element.s3_object_key), safe="/")
url = f"https://{self.s3_bucket_name}.s3.{self.s3_region_name}.amazonaws.com/{encoded_key}"
if self.s3_endpoint_url and self.s3_bucket_name:
@ -661,7 +661,7 @@ class S3Logger(CustomBatchLogger, BaseAWSLLM):
)
# Prepare the URL with percent-encoded object key
encoded_key = quote(s3_object_key, safe="/")
encoded_key = quote(unquote(s3_object_key), safe="/")
url = f"https://{self.s3_bucket_name}.s3.{self.s3_region_name}.amazonaws.com/{encoded_key}"
if self.s3_endpoint_url and self.s3_bucket_name:

View file

@ -1198,7 +1198,7 @@ def test_s3_callback_params_override_empty_dict_is_opt_in():
@pytest.mark.asyncio
@patch("asyncio.create_task")
@patch.object(S3Logger, "_periodic_flush")
@patch("litellm.integrations.s3_v2.CustomBatchLogger.periodic_flush")
async def test_s3_v2_put_url_encodes_special_chars(
mock_periodic_flush, mock_create_task
):