-
Notifications
You must be signed in to change notification settings - Fork 6
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
Add llama370b_instruct_v1 #4
base: main
Are you sure you want to change the base?
Conversation
Named profiles are helpful when you have a lot of different AWS accounts and switch between them. This PR adds support, using the standard AWS environment variable, to allow this to work while also supporting the other configuration methods already implemented in this gem. Also updates docs, changelog, and version bump.
Add Support for AWS Named Profiles
Hi @ErebusBat looks like you'll have to fix conflicts for this PR. Also would you mind trying to add base64 so this issue #5 can be closed and we can link to the PRs. Thanks a lot! |
- Refactor AWS profile configuration
I will rebase to fix conflicts, and look into #5. |
@@ -1,5 +1,5 @@ | |||
# frozen_string_literal: true | |||
|
|||
module RubyAmazonBedrock | |||
VERSION = "0.2.3" | |||
VERSION = "0.2.5" |
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.
Let's keep it to 0.2.4 version
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.
Hi @ErebusBat thanks for the PR, I left a few comments and please make sure rspec
and rubocop
pass. Thanks!
@@ -1,3 +1,7 @@ | |||
## [0.2.4] - 2024-04-25 | |||
|
|||
- Support the use of AWS Named Profiles for authentication |
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.
Hi @ErebusBat let's add all your changes to the same version, please add the following notes to the CHANGELOG
- Support for llama370b_instruct_v1
- Explicitly add base64 gem to dependencies
config.access_key_id = ENV.fetch('AWS_ACCESS_KEY_ID', nil) | ||
config.secret_access_key = ENV.fetch('AWS_SECRET_ACCESS_KEY', nil) | ||
end | ||
You can also use [AWS Named Profiles](https://docs.aws.amazon.com/cli/latest/userguide/cli-configure-files.html#cli-configure-files-format-profile) by passing the `profile` keywoard argument. When using a named profile, specyfing the `region`, `access_key_id` and `access_token` won't be required. |
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.
Don't remove the With Configuration documentation, you can add the AWS Named Profiles note below that.
@@ -8,7 +8,8 @@ | |||
described_class.new( | |||
region: ENV.fetch('AWS_REGION', nil), | |||
access_key_id: ENV.fetch('AWS_ACCESS_KEY_ID', nil), | |||
secret_access_key: ENV.fetch('AWS_SECRET_ACCESS_KEY', nil) | |||
secret_access_key: ENV.fetch('AWS_SECRET_ACCESS_KEY', nil), | |||
profile: ENV.fetch('AWS_PROFILE', nil) |
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.
Please add a new test scenario where the client instantiation is done without the profile
so both cases are covered.
Adds Llama 3 Instruct model
https://docs.aws.amazon.com/bedrock/latest/userguide/model-parameters-meta.html#model-parameters-meta-request-response
This PR bumps the version by two as it build on my other PR #3