-
Couldn't load subscription status.
- Fork 1.2k
fix: Bedrock provider returning None due to inheritance order #3633
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
fix: Bedrock provider returning None due to inheritance order #3633
Conversation
Bedrock provider was returning None for both streaming and non-streaming inference, causing 'NoneType' object has no attribute 'choices' errors. Primary fix: Reorder inheritance to put mixin classes before protocol class in BedrockInferenceAdapter so actual implementations are called. Additional AWS Bedrock API compatibility fixes: - Fix non-streaming: use res["body"].read() instead of next(res["body"]) - Fix streaming: add proper event structure checks and safe access - Disable repetition_penalty (not supported by Bedrock Llama models) Fixes llamastack#3621
|
Hmm, looks like a bit of a non trivial conflict now. Could you please resolve? |
| if sampling_params.repetition_penalty > 0: | ||
| options["repetition_penalty"] = sampling_params.repetition_penalty | ||
| # Note: repetition_penalty is not supported by AWS Bedrock Llama models | ||
| # if sampling_params.repetition_penalty > 0: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why comment out? just leave a note and remove the commented code?
| if "chunk" in event: | ||
| chunk_data = event["chunk"]["bytes"] | ||
| result = json.loads(chunk_data.decode("utf-8")) | ||
| if "generation" in result: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe that if is worth a comment on why we do this?
Bedrock provider was returning None for both streaming and non-streaming inference, causing 'NoneType' object has no attribute 'choices' errors.
Primary fix: Reorder inheritance to put mixin classes before protocol class in BedrockInferenceAdapter so actual implementations are called.
Additional AWS Bedrock API compatibility fixes:
Fixes #3621
Test Plan
Non-Streaming Test
Non-Streaming Success Response
{ "id": "chatcmpl-4a6c063b-53d2-4286-b81d-34ed9c5671e1", "choices": [{ "message": { "role": "assistant", "content": "Hello. What would you like to talk about?", "name": null, "tool_calls": null }, "finish_reason": "stop", "index": 0, "logprobs": null }], "object": "chat.completion", "created": 1759264305, "model": "bedrock-inference/meta.llama3-1-8b-instruct-v1:0" }Streaming Test
Streaming Success Response
Server Log Snippets - Success