Fixes bedrock modelId encoding for Inference Profiles #9123
Conversation
Signed-off-by: omrishiv <327609+omrishiv@users.noreply.github.com>
Signed-off-by: omrishiv <327609+omrishiv@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
omrishiv
changed the title
| import litellm | ||
|
|
||
|
|
||
| def test_encode_model_id_with_inference_profile(): |
There was a problem hiding this comment.
can you add a mock e2e test for completion(), you can mock the http response, test_bedrock_completion.py has some examples
There was a problem hiding this comment.
Not sure I understand, this is addressing an incorrect url encoding. I am happy to update the now failing tests that are asserting the incorrect url. Does that work?
| @@ -274,7 +274,7 @@ def completion( # noqa: PLR0915 | |||
| if modelId is not None: | |||
| modelId = self.encode_model_id(model_id=modelId) | |||
| else: | |||
| modelId = model | |||
| modelId = self.encode_model_id(model_id=model) | |||
There was a problem hiding this comment.
if I pass model = "us.anthropicxxx" would that get encoded too? Is that intended ?
There was a problem hiding this comment.
If I use us.amazon.nova-pro-v1:0 it still works
There was a problem hiding this comment.
what does it get encoded too?
There was a problem hiding this comment.
im concerned doing this for all models might have an un-intended side effect. What do you think @omrishiv ?
There was a problem hiding this comment.
It looks like boto3 actually encodes all the requests:
https://bedrock-runtime.us-east-1.amazonaws.com/model/bedrock%2Fus.meta.llama3-3-70b-instruct-v1%3A0/invoke
This will require updating a lot of the asserts on the tests to update the expected URL, is that ok?
It does work with both completion and acompletion |
There was a problem hiding this comment.
also can this be added to docs https://docs.litellm.ai/docs/providers/bedrock
Do you mean using the instance profile? I'm not sure what needs to be updated? This is just properly conforming the url encoding |
|
@ishaan-jaff @krrishdholakia , please take a look, as discussed, I think this is ready |
krrishdholakia
changed the base branch from
main
to
litellm_dev_03_12_2025_contributor_prs_p2
krrishdholakia
merged commit 2c011d9
into
BerriAI:litellm_dev_03_12_2025_contributor_prs_p2
Title
Make sure to encode bedrock models if they are used as the modelId
Relevant issues
Fixes #8911
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
tests/litellm/directory, Adding at least 1 test is a hard requirement - see detailsmake test-unit)[https://docs.litellm.ai/docs/extras/contributing_code]Type
🐛 Bug Fix
✅ Test
Changes
Encodes the model if it is being used as the modelid