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

Support for aws bedrock using boto3 #1287

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

imZain448
Copy link

@imZain448 imZain448 commented Dec 28, 2024

Add support for aws bedrock using boto3 so that it can be directly used using from_bedrock method

  • add support for bedrock json
  • add support for tools if it supports

this PR adds a new method instructor.from_bedrock which wraps around the client.converse provided by aws boto3 bedrock-runtime api .

client = instructor.from_bedrock(boto3.client('bedrock-runtime', ..credentials..))
response = client.chat.completetions.create(messages, mode, **kwargsl)

Important

Adds AWS Bedrock support using boto3 with new from_bedrock method and Bedrock-specific response handling.

  • Behavior:
    • Adds from_bedrock method in client_bedrock.py to integrate with AWS Bedrock using boto3.
    • Supports BEDROCK_TOOLS and BEDROCK_JSON modes in Mode enum in mode.py.
    • Implements parse_bedrock_json in function_calls.py for JSON parsing specific to Bedrock.
  • Integration:
    • Updates __init__.py to conditionally import from_bedrock if boto3 is available.
    • Adds Provider.BEDROCK in utils.py.
  • Response Handling:
    • Adds handle_bedrock_json and handle_bedrock_tools in process_response.py for processing Bedrock responses.
    • Modifies handle_response_model in process_response.py to include Bedrock modes.

This description was created by Ellipsis for 93e0c4b. It will automatically update as commits are pushed.

@imZain448 imZain448 marked this pull request as ready for review January 26, 2025 09:21
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

👍 Looks good to me! Reviewed everything up to 93e0c4b in 1 minute and 1 seconds

More details
  • Looked at 824 lines of code in 9 files
  • Skipped 0 files when reviewing.
  • Skipped posting 8 drafted comments based on config settings.
1. instructor/client_bedrock.py:30
  • Draft comment:
    Remove print statements used for debugging. Use logging if necessary.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The print statements used for debugging should be removed or replaced with proper logging.
2. instructor/client_bedrock.py:31
  • Draft comment:
    Remove print statements used for debugging. Use logging if necessary.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The print statements used for debugging should be removed or replaced with proper logging.
3. instructor/function_calls.py:299
  • Draft comment:
    Remove print statements used for debugging. Use logging if necessary.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The print statements used for debugging should be removed or replaced with proper logging.
4. instructor/patch.py:186
  • Draft comment:
    Remove print statements used for debugging. Use logging if necessary.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The print statements used for debugging should be removed or replaced with proper logging.
5. instructor/process_response.py:749
  • Draft comment:
    Remove print statements used for debugging. Use logging if necessary.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The print statements used for debugging should be removed or replaced with proper logging.
6. instructor/client_bedrock.py:43
  • Draft comment:
    Assertions should have clear and user-friendly error messages. Consider rephrasing the assertion error messages in the from_bedrock function for better clarity.
  • Reason this comment was not posted:
    Confidence changes required: 80%
    The assertion error messages in from_bedrock function in client_bedrock.py are not formatted well. They should be more descriptive and user-friendly.
7. instructor/client_bedrock.py:31
  • Draft comment:
    Avoid using print statements for debugging in production code. Consider using a logging framework instead.
  • Reason this comment was not posted:
    Confidence changes required: 80%
    The print statements in handle_bedrock_json function in client_bedrock.py are not necessary for production code and should be removed or replaced with proper logging.
8. instructor/function_calls.py:299
  • Draft comment:
    Avoid using print statements for debugging in production code. Consider using a logging framework instead.
  • Reason this comment was not posted:
    Confidence changes required: 80%
    The print statement in parse_bedrock_json function in function_calls.py is not necessary for production code and should be removed or replaced with proper logging.

Workflow ID: wflow_EHi0ojlV6htlekmA


You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@imZain448
Copy link
Author

Hi Please review this PR

@ivanleomk
Copy link
Collaborator

Hi @imZain448 thanks for submitting the PR, I'll get aws bedrock setup this week and will test this out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants