Skip to content

Commit 321d529

Browse files
s3_endpoint_url returned 404 (#14559)
* added spend metrics * feat: Add Spend metrics in datadog * fix: lint errors * fix: s3 endpoint url logging * fixed lint errors * remove from branch This reverts commit e123cae. * Remove from branch This reverts commit e694cc1. * remove "added spend metrics" This reverts commit 6156590.
1 parent cebacd6 commit 321d529

File tree

2 files changed

+175
-23
lines changed

2 files changed

+175
-23
lines changed

litellm/integrations/s3_v2.py

Lines changed: 39 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ async def async_log_success_event(self, kwargs, response_obj, start_time, end_ti
203203
start_time=start_time,
204204
end_time=end_time,
205205
)
206-
206+
207207
async def async_log_failure_event(self, kwargs, response_obj, start_time, end_time):
208208
await self._async_log_event_base(
209209
kwargs=kwargs,
@@ -212,7 +212,6 @@ async def async_log_failure_event(self, kwargs, response_obj, start_time, end_ti
212212
end_time=end_time,
213213
)
214214
pass
215-
216215

217216
async def _async_log_event_base(self, kwargs, response_obj, start_time, end_time):
218217
try:
@@ -242,7 +241,6 @@ async def _async_log_event_base(self, kwargs, response_obj, start_time, end_time
242241
verbose_logger.exception(f"s3 Layer Error - {str(e)}")
243242
pass
244243

245-
246244
async def async_upload_data_to_s3(
247245
self, batch_logging_element: s3BatchLoggingElement
248246
):
@@ -277,8 +275,14 @@ async def async_upload_data_to_s3(
277275
# Prepare the URL
278276
url = f"https://{self.s3_bucket_name}.s3.{self.s3_region_name}.amazonaws.com/{batch_logging_element.s3_object_key}"
279277

280-
if self.s3_endpoint_url:
281-
url = self.s3_endpoint_url + "/" + batch_logging_element.s3_object_key
278+
if self.s3_endpoint_url and self.s3_bucket_name:
279+
url = (
280+
self.s3_endpoint_url
281+
+ "/"
282+
+ self.s3_bucket_name
283+
+ "/"
284+
+ batch_logging_element.s3_object_key
285+
)
282286

283287
# Convert JSON to string
284288
json_string = safe_dumps(batch_logging_element.payload)
@@ -420,8 +424,14 @@ def upload_data_to_s3(self, batch_logging_element: s3BatchLoggingElement):
420424
# Prepare the URL
421425
url = f"https://{self.s3_bucket_name}.s3.{self.s3_region_name}.amazonaws.com/{batch_logging_element.s3_object_key}"
422426

423-
if self.s3_endpoint_url:
424-
url = self.s3_endpoint_url + "/" + batch_logging_element.s3_object_key
427+
if self.s3_endpoint_url and self.s3_bucket_name:
428+
url = (
429+
self.s3_endpoint_url
430+
+ "/"
431+
+ self.s3_bucket_name
432+
+ "/"
433+
+ batch_logging_element.s3_object_key
434+
)
425435

426436
# Convert JSON to string
427437
json_string = safe_dumps(batch_logging_element.payload)
@@ -462,14 +472,13 @@ def upload_data_to_s3(self, batch_logging_element: s3BatchLoggingElement):
462472
except Exception as e:
463473
verbose_logger.exception(f"Error uploading to s3: {str(e)}")
464474

465-
466475
async def _download_object_from_s3(self, s3_object_key: str) -> Optional[dict]:
467476
"""
468477
Download and parse JSON object from S3.
469-
478+
470479
Args:
471480
s3_object_key: The S3 object key to download
472-
481+
473482
Returns:
474483
Optional[dict]: The parsed JSON object or None if not found/error
475484
"""
@@ -481,7 +490,7 @@ async def _download_object_from_s3(self, s3_object_key: str) -> Optional[dict]:
481490
from botocore.awsrequest import AWSRequest
482491
except ImportError:
483492
raise ImportError("Missing boto3 to call S3. Run 'pip install boto3'.")
484-
493+
485494
try:
486495
from litellm.litellm_core_utils.asyncify import asyncify
487496

@@ -506,8 +515,14 @@ async def _download_object_from_s3(self, s3_object_key: str) -> Optional[dict]:
506515
# Prepare the URL
507516
url = f"https://{self.s3_bucket_name}.s3.{self.s3_region_name}.amazonaws.com/{s3_object_key}"
508517

509-
if self.s3_endpoint_url:
510-
url = self.s3_endpoint_url + "/" + s3_object_key
518+
if self.s3_endpoint_url and self.s3_bucket_name:
519+
url = (
520+
self.s3_endpoint_url
521+
+ "/"
522+
+ self.s3_bucket_name
523+
+ "/"
524+
+ s3_object_key
525+
)
511526

512527
# Prepare the request for GET operation
513528
# For GET requests, we need x-amz-content-sha256 with hash of empty string
@@ -533,12 +548,14 @@ async def _download_object_from_s3(self, s3_object_key: str) -> Optional[dict]:
533548
response = await self.async_httpx_client.get(url, headers=signed_headers)
534549

535550
if response.status_code != 200:
536-
verbose_logger.exception("S3 object not found, saw response=", response.text)
551+
verbose_logger.exception(
552+
"S3 object not found, saw response=", response.text
553+
)
537554
return None
538-
555+
539556
# Parse JSON response
540557
return response.json()
541-
558+
542559
except Exception as e:
543560
verbose_logger.exception(f"Error downloading from S3: {str(e)}")
544561
return None
@@ -551,11 +568,11 @@ async def get_proxy_server_request_from_cold_storage_with_object_key(
551568
Get the proxy server request from cold storage
552569
553570
Allows fetching a dict of the proxy server request from s3 or GCS bucket.
554-
571+
555572
Args:
556573
request_id: The unique request ID to search for
557574
start_time: The start time of the request (datetime or ISO string)
558-
575+
559576
Returns:
560577
Optional[dict]: The request data dictionary or None if not found
561578
"""
@@ -564,5 +581,7 @@ async def get_proxy_server_request_from_cold_storage_with_object_key(
564581
downloaded_object = await self._download_object_from_s3(object_key)
565582
return downloaded_object
566583
except Exception as e:
567-
verbose_logger.exception(f"Error retrieving object {object_key} from cold storage: {str(e)}")
568-
return None
584+
verbose_logger.exception(
585+
f"Error retrieving object {object_key} from cold storage: {str(e)}"
586+
)
587+
return None

tests/test_litellm/integrations/test_s3_v2.py

Lines changed: 136 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
class TestS3V2UnitTests:
1212
"""Test that S3 v2 integration only uses safe_dumps and not json.dumps"""
13+
1314
def test_s3_v2_source_code_analysis(self):
1415
"""Test that S3 v2 source code only imports and uses safe_dumps"""
1516
import inspect
@@ -18,7 +19,139 @@ def test_s3_v2_source_code_analysis(self):
1819

1920
# Get the source code of the s3_v2 module
2021
source_code = inspect.getsource(s3_v2)
21-
22+
2223
# Verify that json.dumps is not used directly in the code
23-
assert "json.dumps(" not in source_code, \
24-
"S3 v2 should not use json.dumps directly"
24+
assert (
25+
"json.dumps(" not in source_code
26+
), "S3 v2 should not use json.dumps directly"
27+
28+
@patch('asyncio.create_task')
29+
@patch('litellm.integrations.s3_v2.CustomBatchLogger.periodic_flush')
30+
def test_s3_v2_endpoint_url(self, mock_periodic_flush, mock_create_task):
31+
"""testing s3 endpoint url"""
32+
from unittest.mock import AsyncMock, MagicMock
33+
from litellm.types.integrations.s3_v2 import s3BatchLoggingElement
34+
35+
# Mock periodic_flush and create_task to prevent async task creation during init
36+
mock_periodic_flush.return_value = None
37+
mock_create_task.return_value = None
38+
39+
# Mock response for all tests
40+
mock_response = MagicMock()
41+
mock_response.status_code = 200
42+
mock_response.raise_for_status = MagicMock()
43+
44+
# Create a test batch logging element
45+
test_element = s3BatchLoggingElement(
46+
s3_object_key="2025-09-14/test-key.json",
47+
payload={"test": "data"},
48+
s3_object_download_filename="test-file.json"
49+
)
50+
51+
# Test 1: Custom endpoint URL with bucket name
52+
s3_logger = S3Logger(
53+
s3_bucket_name="test-bucket",
54+
s3_endpoint_url="https://s3.amazonaws.com",
55+
s3_aws_access_key_id="test-key",
56+
s3_aws_secret_access_key="test-secret",
57+
s3_region_name="us-east-1"
58+
)
59+
60+
s3_logger.async_httpx_client = AsyncMock()
61+
s3_logger.async_httpx_client.put.return_value = mock_response
62+
63+
asyncio.run(s3_logger.async_upload_data_to_s3(test_element))
64+
65+
call_args = s3_logger.async_httpx_client.put.call_args
66+
assert call_args is not None
67+
url = call_args[0][0]
68+
expected_url = "https://s3.amazonaws.com/test-bucket/2025-09-14/test-key.json"
69+
assert url == expected_url, f"Expected URL {expected_url}, got {url}"
70+
71+
# Test 2: MinIO-compatible endpoint
72+
s3_logger_minio = S3Logger(
73+
s3_bucket_name="litellm-logs",
74+
s3_endpoint_url="https://minio.example.com:9000",
75+
s3_aws_access_key_id="minio-key",
76+
s3_aws_secret_access_key="minio-secret",
77+
s3_region_name="us-east-1"
78+
)
79+
80+
s3_logger_minio.async_httpx_client = AsyncMock()
81+
s3_logger_minio.async_httpx_client.put.return_value = mock_response
82+
83+
asyncio.run(s3_logger_minio.async_upload_data_to_s3(test_element))
84+
85+
call_args_minio = s3_logger_minio.async_httpx_client.put.call_args
86+
assert call_args_minio is not None
87+
url_minio = call_args_minio[0][0]
88+
expected_minio_url = "https://minio.example.com:9000/litellm-logs/2025-09-14/test-key.json"
89+
assert url_minio == expected_minio_url, f"Expected MinIO URL {expected_minio_url}, got {url_minio}"
90+
91+
# Test 3: Custom endpoint without bucket name (should fall back to default)
92+
s3_logger_no_bucket = S3Logger(
93+
s3_endpoint_url="https://s3.amazonaws.com",
94+
s3_aws_access_key_id="test-key",
95+
s3_aws_secret_access_key="test-secret",
96+
s3_region_name="us-east-1"
97+
)
98+
99+
s3_logger_no_bucket.async_httpx_client = AsyncMock()
100+
s3_logger_no_bucket.async_httpx_client.put.return_value = mock_response
101+
102+
asyncio.run(s3_logger_no_bucket.async_upload_data_to_s3(test_element))
103+
104+
call_args_no_bucket = s3_logger_no_bucket.async_httpx_client.put.call_args
105+
assert call_args_no_bucket is not None
106+
url_no_bucket = call_args_no_bucket[0][0]
107+
# Should use default S3 URL format when bucket is missing (bucket becomes None in URL)
108+
assert "s3.us-east-1.amazonaws.com" in url_no_bucket
109+
assert "https://" in url_no_bucket
110+
# Should not include the custom endpoint since bucket is missing
111+
assert "https://s3.amazonaws.com/" not in url_no_bucket
112+
113+
# Test 4: Sync upload method with custom endpoint
114+
s3_logger_sync = S3Logger(
115+
s3_bucket_name="sync-bucket",
116+
s3_endpoint_url="https://custom.s3.endpoint.com",
117+
s3_aws_access_key_id="sync-key",
118+
s3_aws_secret_access_key="sync-secret",
119+
s3_region_name="us-east-1"
120+
)
121+
122+
mock_sync_client = MagicMock()
123+
mock_sync_client.put.return_value = mock_response
124+
125+
with patch('litellm.integrations.s3_v2._get_httpx_client', return_value=mock_sync_client):
126+
s3_logger_sync.upload_data_to_s3(test_element)
127+
128+
call_args_sync = mock_sync_client.put.call_args
129+
assert call_args_sync is not None
130+
url_sync = call_args_sync[0][0]
131+
expected_sync_url = "https://custom.s3.endpoint.com/sync-bucket/2025-09-14/test-key.json"
132+
assert url_sync == expected_sync_url, f"Expected sync URL {expected_sync_url}, got {url_sync}"
133+
134+
# Test 5: Download method with custom endpoint
135+
s3_logger_download = S3Logger(
136+
s3_bucket_name="download-bucket",
137+
s3_endpoint_url="https://download.s3.endpoint.com",
138+
s3_aws_access_key_id="download-key",
139+
s3_aws_secret_access_key="download-secret",
140+
s3_region_name="us-east-1"
141+
)
142+
143+
mock_download_response = MagicMock()
144+
mock_download_response.status_code = 200
145+
mock_download_response.json = MagicMock(return_value={"downloaded": "data"})
146+
s3_logger_download.async_httpx_client = AsyncMock()
147+
s3_logger_download.async_httpx_client.get.return_value = mock_download_response
148+
149+
result = asyncio.run(s3_logger_download._download_object_from_s3("2025-09-14/download-test-key.json"))
150+
151+
call_args_download = s3_logger_download.async_httpx_client.get.call_args
152+
assert call_args_download is not None
153+
url_download = call_args_download[0][0]
154+
expected_download_url = "https://download.s3.endpoint.com/download-bucket/2025-09-14/download-test-key.json"
155+
assert url_download == expected_download_url, f"Expected download URL {expected_download_url}, got {url_download}"
156+
157+
assert result == {"downloaded": "data"}

0 commit comments

Comments
 (0)