Skip to content

Feature/Parameterize Bedrock model ID in CloudFormation templates#238

Open
oussamahansal wants to merge 2 commits intomainfrom
parameterize-bedrock-model
Open

Feature/Parameterize Bedrock model ID in CloudFormation templates#238
oussamahansal wants to merge 2 commits intomainfrom
parameterize-bedrock-model

Conversation

@oussamahansal
Copy link
Copy Markdown
Collaborator

Description of changes:

  • Replace hardcoded anthropic.claude-sonnet-4-6 model references with a configurable BedrockModelId parameter across CloudFormation templates.
  • Users can now specify which Bedrock model to use without editing template source files.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

],
"Resource": [
{
"Fn::Sub": "arn:${AWS::Partition}:bedrock:*:${AWS::AccountId}:inference-profile/*.anthropic.*"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This doesn't work for the workshop, because the workshop ModelId parameter can also accept a profile, which already includes the prefix. We need this profile to carry all the way through, because a) Many models will now only work when invoked with a profile - sonnet-4-6, for example, b) the workshop's cached responses are keyed for the specific modelId/profile that is suplpied.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I believe removing the *. would work, right?

Copy link
Copy Markdown
Contributor

@iansrobinson iansrobinson left a comment

Choose a reason for hiding this comment

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

My preference would be for ModelId to accept a model ID or profile. Many models, sonnet-4-6 for example, have to be invoked via a profile. Therefore, I suspect the configuration of environment variables will fail unless supplied with a profile.

In the end, I liked the use of *.anthropic.* in the IAM profiles. It's not as locked to specific models as we'd like, but it works in the face of the inconsistencies in Bedrock.

"Default": "https://github.com/awslabs/graphrag-toolkit/releases/latest/download/lexical-graph-examples-latest.zip"
},
"BedrockModelId": {
"Description": "Bedrock model ID used for extraction, response, and evaluation LLMs",
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
"Description": "Bedrock model ID used for extraction, response, and evaluation LLMs",
"Description": "Bedrock model ID or inference profile used for extraction, response, and evaluation LLMs",

"Default": "https://github.com/awslabs/graphrag-toolkit/releases/latest/download/lexical-graph-examples-latest.zip"
},
"BedrockModelId": {
"Description": "Bedrock model ID used for extraction, response, and evaluation LLMs",
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
"Description": "Bedrock model ID used for extraction, response, and evaluation LLMs",
"Description": "Bedrock model ID or inference profile used for extraction, response, and evaluation LLMs",

"Default": "https://github.com/awslabs/graphrag-toolkit/releases/latest/download/lexical-graph-examples-latest.zip"
},
"BedrockModelId": {
"Description": "Bedrock model ID used for extraction, response, and evaluation LLMs",
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
"Description": "Bedrock model ID used for extraction, response, and evaluation LLMs",
"Description": "Bedrock model ID or inference profile used for extraction, response, and evaluation LLMs",

"Default": "https://github.com/awslabs/graphrag-toolkit/releases/latest/download/lexical-graph-examples-latest.zip"
},
"BedrockModelId": {
"Description": "Bedrock model ID used for extraction, response, and evaluation LLMs",
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
"Description": "Bedrock model ID used for extraction, response, and evaluation LLMs",
"Description": "Bedrock model ID or inference profile used for extraction, response, and evaluation LLMs",

"Default": "https://github.com/awslabs/graphrag-toolkit/releases/latest/download/lexical-graph-examples-latest.zip"
},
"BedrockModelId": {
"Description": "Bedrock model ID used for extraction, response, and evaluation LLMs",
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
"Description": "Bedrock model ID used for extraction, response, and evaluation LLMs",
"Description": "Bedrock model ID or inference profile used for extraction, response, and evaluation LLMs",

"Default": "https://github.com/awslabs/graphrag-toolkit/releases/latest/download/lexical-graph-examples-latest.zip"
},
"BedrockModelId": {
"Description": "Bedrock model ID used for extraction, response, and evaluation LLMs",
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
"Description": "Bedrock model ID used for extraction, response, and evaluation LLMs",
"Description": "Bedrock model ID or inference profile used for extraction, response, and evaluation LLMs",

},
{
"Fn::Sub": "arn:${AWS::Partition}:bedrock:*:${AWS::AccountId}:inference-profile/*.anthropic.*"
"Fn::Sub": "arn:${AWS::Partition}:bedrock:*:${AWS::AccountId}:inference-profile/*.${BedrockModelId}"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
"Fn::Sub": "arn:${AWS::Partition}:bedrock:*:${AWS::AccountId}:inference-profile/*.${BedrockModelId}"
"Fn::Sub": "arn:${AWS::Partition}:bedrock:*:${AWS::AccountId}:inference-profile/*${BedrockModelId}"

},
{
"Fn::Sub": "arn:${AWS::Partition}:bedrock:*:${AWS::AccountId}:inference-profile/*.anthropic.*"
"Fn::Sub": "arn:${AWS::Partition}:bedrock:*:${AWS::AccountId}:inference-profile/*.${BedrockModelId}"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
"Fn::Sub": "arn:${AWS::Partition}:bedrock:*:${AWS::AccountId}:inference-profile/*.${BedrockModelId}"
"Fn::Sub": "arn:${AWS::Partition}:bedrock:*:${AWS::AccountId}:inference-profile/*${BedrockModelId}"

],
"Resource": [
{
"Fn::Sub": "arn:${AWS::Partition}:bedrock:*:${AWS::AccountId}:inference-profile/*.anthropic.*"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I believe removing the *. would work, right?

},
{
"Fn::Sub": "arn:${AWS::Partition}:bedrock:*:${AWS::AccountId}:inference-profile/*.anthropic.*"
"Fn::Sub": "arn:${AWS::Partition}:bedrock:*:${AWS::AccountId}:inference-profile/*.${BedrockModelId}"
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
"Fn::Sub": "arn:${AWS::Partition}:bedrock:*:${AWS::AccountId}:inference-profile/*.${BedrockModelId}"
"Fn::Sub": "arn:${AWS::Partition}:bedrock:*:${AWS::AccountId}:inference-profile/${BedrockModelId}"

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I don't think we need the wildcard for profiles, since the profile passed in would contain the prefix.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We might need to go with something like *${BedrockModelId}*?

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.

3 participants