Skip to content
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

feat: Add Amazon Bedrock chat model support #333

Merged
merged 19 commits into from
Feb 8, 2024
Merged

feat: Add Amazon Bedrock chat model support #333

merged 19 commits into from
Feb 8, 2024

Conversation

vblagoje
Copy link
Member

@vblagoje vblagoje commented Feb 2, 2024

Why:

Introduces significant upgrades to the Amazon Bedrock integration by adding functionality for chat model generation. The motivation behind these changes is to support conversational AI and chatbot applications that leverage Amazon Bedrock's machine learning capabilities. This enhancement expands the project's features, allowing users to interact with AI models in a chat-like manner and enable streaming of responses for real-time interaction.

What:

  • Added AmazonBedrockChatGenerator: Incorporates the ability for creating chat-like interactions with AI models, including streaming response support.
  • Introduced BedrockModelChatAdapter base class: Establishes a foundation for various Amazon Bedrock chat model adapters.
  • Added AnthropicClaudeChatAdapter and MetaLlama2ChatAdapter: Implements specific adapters for the Anthropic Claude and Meta Llama models, respectively.
  • Updated generator.py: Adjusted import statement to reflect the integration's new module structure.
  • Added tests: Includes test cases in test_amazon_chat_bedrock.py to ensure the chat functionality works as expected.
  • Modified __all__ directive in __init__.py: Expanded the list of imports to include AmazonBedrockChatGenerator.

How can it be used:

  • Chat Generation:

    from haystack_integrations.components.generators.amazon_bedrock import AmazonBedrockChatGenerator
    from haystack.dataclasses import ChatMessage
    
    # Initialize the chat generator with an appropriate model.
    chat_gen = AmazonBedrockChatGenerator(model="anthropic.claude-v2")
    
    # Define a list of ChatMessage instances representing the conversation.
    messages = [
        ChatMessage.from_system("You are a helpful assistant."),
        ChatMessage.from_user("What's two plus two?")
    ]
    
    # Generate a response from the model.
    response = chat_gen.run(messages=messages)
  • Streaming Responses:

    # Define a callback to handle streaming chunks
    def streaming_callback(chunk):
        # Handle streaming chunk (e.g., display in UI)
    
    # Initialize the generator with streaming enabled
    chat_gen_stream = AmazonBedrockChatGenerator(
        model="anthropic.claude-v2",
        streaming_callback=streaming_callback
    )
    
    # Generate a streamed response from the model
    chat_gen_stream.run(messages=messages)

How did you test it:

The testing process involves both unit and integration tests. The unit tests, found in test_amazon_chat_bedrock.py, check the functionality of different components such as model adapters and chat generators. These tests ensure that the classes behave as expected when provided with mock data and simulated AWS sessions. Integration tests would involve the actual AWS environment and interaction with the Bedrock API, ensuring end-to-end functionality.

Notes for the reviewer:

  • Regard for Specific Model Limitations: Each model adapter addresses the nuances and constraints of its respective language model (e.g., token limits).
  • Modification to Import Statements: The structural changes to the project directories are reflected in the updated import statements. Review the module paths for accuracy.
  • Ensure Environment Configuration: As the enhancements involve interaction with the AWS services, ensure environmental variables or credentials are configured properly.
  • Review Streaming Callbacks: The streaming functionality introduces asynchronous behavior. Review the handling logic within callbacks to ensure proper sequence and error handling.
  • Acknowledgement of API Changes: The introduced changes can affect downstream dependencies, especially in how the generator classes are imported and utilized. Compatibility checks with dependent modules might be necessary.

@vblagoje vblagoje requested a review from a team as a code owner February 2, 2024 15:56
@vblagoje vblagoje requested review from davidsbatista and removed request for a team February 2, 2024 15:56
@github-actions github-actions bot added the type:documentation Improvements or additions to documentation label Feb 7, 2024
Copy link
Contributor

@davidsbatista davidsbatista left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just some small remarks, nothing impeding - I will still play around with both models and have another look at the tests

"""Extracts the responses from the Amazon Bedrock response."""
return self._extract_messages_from_response(response_body)

def get_stream_responses(self, stream, stream_handler: Callable[[StreamingChunk], None]) -> List[str]:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we can add a type to stream ?

responses = ["".join(tokens).lstrip()]
return responses

def _update_params(self, target_dict: Dict[str, Any], updates_dict: Dict[str, Any]) -> None:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the self is never used here, seems like this can be a static method @statichmethod

Note that the AWS credentials are not required if the AWS environment is configured correctly. These are loaded
automatically from the environment or the AWS configuration file and do not need to be provided explicitly via
the constructor.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: from my experience, I suspect three minimum mandatory parameters need to be configured: aws_access_key_id, aws_session_token, aws_region_name, maybe a sentence in the docstring mention that

)
raise AmazonBedrockConfigurationError(msg) from exception

model_adapter_cls = self.get_model_adapter(model=model)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: maybe first check if the given string is a valid AWS model before setting up a connection with AWS?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's what get_model_adapterdoes. If not found we'll raise AmazonBedrockConfigurationError

@davidsbatista
Copy link
Contributor

I've noticed that the code format for some files has changed.

I don't know if this is intended or if this is a misconfiguration on either the previous commit or this new commit - I am just raising that I've noticed it, but I don't know which formatting style is correct.

@davidsbatista
Copy link
Contributor

after talking with @vblagoje, we will revert the code formatting in a separate PR

@vblagoje vblagoje requested a review from dfokina February 8, 2024 11:18
Copy link
Contributor

@davidsbatista davidsbatista left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@vblagoje vblagoje merged commit dd92c1b into main Feb 8, 2024
10 checks passed
@vblagoje vblagoje deleted the amzn_chat branch February 8, 2024 13:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integration:amazon-bedrock type:documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants