fix(bedrock): preserve provider upload_url in sync create_file path#21628
Closed
Chesars wants to merge 1 commit intoBerriAI:mainfrom
Closed
fix(bedrock): preserve provider upload_url in sync create_file path#21628Chesars wants to merge 1 commit intoBerriAI:mainfrom
Chesars wants to merge 1 commit intoBerriAI:mainfrom
Conversation
The sync create_file path unconditionally overwrote litellm_params["upload_url"] with api_base, discarding the correct URL set by Bedrock's transform_create_file_request. This caused the returned file_id to point to a different S3 key than the one the file was actually uploaded to, because get_complete_file_url generates a new UUID on each call. Add a guard so api_base is only used as fallback when the provider has not already set upload_url. Fixes BerriAI#21546
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Greptile SummaryFixed a UUID mismatch bug in Bedrock's sync Key Changes:
Note: The async path ( Confidence Score: 5/5
|
| Filename | Overview |
|---|---|
| litellm/llms/custom_httpx/llm_http_handler.py | Added conditional check to preserve provider-set upload_url instead of unconditionally overwriting with api_base |
| tests/test_litellm/llms/bedrock/files/test_bedrock_files_integration.py | Added regression test verifying sync create_file preserves provider's upload_url from litellm_params |
Sequence Diagram
sequenceDiagram
participant Client
participant HTTPHandler
participant BedrockTransform
participant S3
Note over Client,S3: Before Fix: UUID Mismatch Bug
Client->>HTTPHandler: create_file(sync)
HTTPHandler->>BedrockTransform: get_complete_file_url()
BedrockTransform-->>HTTPHandler: api_base (UUID-1)
HTTPHandler->>BedrockTransform: transform_create_file_request()
BedrockTransform->>BedrockTransform: generate new UUID-2
BedrockTransform->>BedrockTransform: set litellm_params["upload_url"] = UUID-2
BedrockTransform-->>HTTPHandler: presigned request (UUID-2)
HTTPHandler->>S3: PUT to UUID-2 ✓
HTTPHandler->>HTTPHandler: OVERWRITES upload_url = api_base (UUID-1) ✗
HTTPHandler->>BedrockTransform: transform_create_file_response(UUID-1)
BedrockTransform-->>Client: file_id points to UUID-1 (wrong!)
Note over Client,S3: After Fix: UUID Preserved
Client->>HTTPHandler: create_file(sync)
HTTPHandler->>BedrockTransform: get_complete_file_url()
BedrockTransform-->>HTTPHandler: api_base (UUID-1)
HTTPHandler->>BedrockTransform: transform_create_file_request()
BedrockTransform->>BedrockTransform: generate new UUID-2
BedrockTransform->>BedrockTransform: set litellm_params["upload_url"] = UUID-2
BedrockTransform-->>HTTPHandler: presigned request (UUID-2)
HTTPHandler->>S3: PUT to UUID-2 ✓
HTTPHandler->>HTTPHandler: preserves upload_url = UUID-2 ✓
HTTPHandler->>BedrockTransform: transform_create_file_response(UUID-2)
BedrockTransform-->>Client: file_id points to UUID-2 (correct!)
Last reviewed commit: abe7eb6
Collaborator
Author
|
Closing as superseded by #21650. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Relevant issues
Fixes #21546
Pre-Submission checklist
tests/litellm/directory, Adding at least 1 test is a hard requirement - see detailsmake test-unit@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewType
🐛 Bug Fix
Changes
The sync
create_filepath inllm_http_handler.pyunconditionally overwriteslitellm_params["upload_url"]withapi_base(line 3018). For Bedrock,get_complete_file_urlgenerates a newuuid.uuid4()on each call, soapi_base(from the first call) and the URL used for the actual S3 upload (from the second call insidetransform_create_file_request) contain different UUIDs. The file is uploaded to the correct key (UUID-2) but the returnedfile_idpoints to the stale key (UUID-1).The fix adds a guard so
api_baseis only used as a fallback when the provider has not already setupload_urlinlitellm_params. This preserves the correct URL that Bedrock writes atbedrock/files/transformation.py:370.