-
Notifications
You must be signed in to change notification settings - Fork 241
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
Updated bedrock.py to handle case of deploying it over Lambda function with attached role #85
Conversation
thanks @irshadc , for the PR. the changes make sense to me. cc @marklysze, have you tried/tested this locally? please let me know, thanks. |
I'm afraid I haven't yet. @irshadc, are you able to run the pre-commit checks, I think the code formatting will need an update. more info here I'm not familiar with how we could test this as it seems to require running in AWS with an attached role - @irshadc any guidance here? |
@irshadc could you give me permission to your fork and I can help you fix the code format issue. Thank you! |
@marklysze I gave you access to forked-repo. Please update the code. As it is first time I am sending a patch here. Yes, You need to have AWS account to test it further. I am using my forked repo in requirements.txt to deploy it in AWS Lambda function. |
…ces. Current approach only consider providing access credentials as part of parameter. This approach have security concerns in managing access keys and secrets. AWS Best practices recommends to use Attached IAM Role with Managed AWS services like AWS Lambda, EC2, ECS Task etc. I have added condition to check if access_key is not present it should try accessing attached instance role directly to provide bedrock-runtime.
@irshadc thanks for your contribution. Could you share your Discord handle and/or email address with @qingyun-wu for CLA? |
Thanks @irshadc, I've updated based on the |
Thanks @irshadc, I tested in an AWS Lambda function using your repo in the requirements.txt and it worked:
I also tested using the original |
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.
Tested with an AWS Lambda function and it worked as expected. Also tested with original credentials and still worked. Thanks @irshadc
Note: I've added a comment in the Bedrock documentation that this is supported. |
|
Thanks a lot @marklysze, My email id is [email protected] I am not on discord. |
[Docs] Generate Documentation from live objects instead of parsing the source code
#84 [Bug]: Code doesn't work on AWS Lambda function when deployed with attached Role to access bedrock service.
Description
Code was not working on AWS Lambda function when deployed with attached Role to access bedrock service.
Why are these changes needed?
When passed credentials locally or via AWS profile, it works fine. However, it doesn't pick automatically session when deployed on AWS with managed services (eg: AWS Lambda function, Amazon ECS task, etc.) or Amazon EC2 with attached role. Although it is recommended and more secure way than creating access credentials.
I have made changes to build bedrock-runtime client directly using boto3 which uses attached Role to managed AWS service.
This is more secure way to running code over AWS. Tested it with AWS Lambda function and ECS container task.
Key Changes:
Related issue number
Fixes #84
Checks
Testing Details