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 new target class for AWS Bedrock Anthropic Claude models #699

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

Conversation

kmarsh77
Copy link

@kmarsh77 kmarsh77 commented Feb 7, 2025

Description

Adding a new target class for AWS Bedrock Anthropic Claude models. It will only work for Anthropic Claude models as the request body is specific to those, but it should be easy to modify the class for use with other Bedrock models.

boto3 Python library is used for sending requests. Local AWS credentials are used for authentication, typically stored in ~/.aws.

Tests and Documentation

Description of target class and parameters is provided in the class file, aws_bedrock_claude_target.py. Unit tests are provided in tests/unit/test_aws_bedrock_claude_target.py.

@kmarsh77
Copy link
Author

kmarsh77 commented Feb 7, 2025 via email

Copy link
Contributor

@romanlutz romanlutz left a comment

Choose a reason for hiding this comment

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

Overall this looks very good! I left a few comments, and you likely have complaints from our pre-commit hooks. You can run pre-commit run --all-files to fix that locally.

use of aws_bedrock_claude_target.py target class requires the boto3 library
Deleting unnecessary commented out line
Adding support for multi-turn
@kmarsh77
Copy link
Author

I ran pre-commit run --all-files locally and pushed those changes to my fork.

The only error I couldn't fix is the following:
pyrit/prompt_target/aws_bedrock_claude_chat_target.py:150: error: Dict entry 1 has incompatible type "str": "dict[str, str]"; expected "str": "str" [dict-item]

This is referring to case where an image is sent in a message; the expected format for AWS Bedrock Claude is:
{ "type": "image", "source": { "type": "base64", "media_type": "image/jpeg", "data": "content image bytes" } }

Therefore I'm not sure how to fix this error.

Copy link
Contributor

@romanlutz romanlutz left a comment

Choose a reason for hiding this comment

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

This is looking great! Just a few small things. We're also getting an account so that we can run this in integration tests soon.



@pytest.mark.asyncio
async def test_send_prompt_async(aws_target, mock_prompt_request):
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to skip these if boto3 isn't installed. For HuggingFace chat target we do

def is_torch_installed():
    try:
        import torch  # noqa: F401

        return True
    except ModuleNotFoundError:
        return False

@pytest.mark.skipif(not is_torch_installed(), reason="torch is not installed")
...

Maybe you can do the same? Otherwise it'll fail in the cases where boto3 isn't installed.

Similarly, you can't import boto3 at the top of the file, but you need to import inside a try-except block inside your target constructor. See hugging_face_chat_target.py for an example where we do this with torch.

Copy link
Author

Choose a reason for hiding this comment

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

I've made the suggested changes and pushed them to my fork, however note that pre-commit is now complaining that "'boto3' imported but unused". Let me know if I can fix it.

Copy link
Contributor

Choose a reason for hiding this comment

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

We added the # noqa: F401 for that (see in the snippet above)

Copy link
Author

Choose a reason for hiding this comment

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

Added this for the imports in both files, also added the necessary if TYPE_CHECKING block at top of target file.

import boto3 # noqa: F401
from botocore.exceptions import ClientError # noqa: F401
except ModuleNotFoundError as e:
logger.error("Could not import boto. You may need to install it via 'pip install pyrit[all]'")
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
logger.error("Could not import boto. You may need to install it via 'pip install pyrit[all]'")
logger.error("Could not import boto. You may need to install it via 'pip install pyrit[all] or pyrit[aws]'")

@romanlutz
Copy link
Contributor

I'm seeing odd test errors related to the boto3 import. Stared at it for 10 minutes and can't quite make sense of it. Probably easier to repro if you have the branch locally. Lmk if you are having trouble.

@romanlutz
Copy link
Contributor

Related: lots of cloud providers like Azure, Google, and other local model providers like ollama use openai compatible APIs so we don't need anything custom. Looks like AWS is a bit behind on that front. I only see workarounds where people have deployed additional things to make that conversion work. Whenever they do become compatible I'd love to just move over to OpenAIChatTarget, though.

@kmarsh77
Copy link
Author

@romanlutz I am actually unable to replicate the import error relating to boto3, could you please share the command you are using?

@kmarsh77
Copy link
Author

@romanlutz alright I fixed the one error which was invalid value for converted_value_data_type in test_validate_request_invalid_data_type().

The other error is being caused by test_send_prompt_async() and test_complete_chat_async() not seeing an import for boto3 in the target class file (because the import is within try/except block in the initialization function rather than at the top of the file).

I'm not sure how to get around this but will think on it. Let me know if you have any ideas.

@romanlutz
Copy link
Contributor

@romanlutz alright I fixed the one error which was invalid value for converted_value_data_type in test_validate_request_invalid_data_type().

The other error is being caused by test_send_prompt_async() and test_complete_chat_async() not seeing an import for boto3 in the target class file (because the import is within try/except block in the initialization function rather than at the top of the file).

I'm not sure how to get around this but will think on it. Let me know if you have any ideas.

I'll try to repro it sometime this week. Thank you!

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