-
Notifications
You must be signed in to change notification settings - Fork 493
Add enableBedrockGlobalInference #945
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
base: v3
Are you sure you want to change the base?
Conversation
@mludvig Thank you for your contribution! Could you please also change the unit test for |
Added test for global inference:
|
@mludvig It looks like CDK test failed. Could you please fix it? |
@Yukinobu-Mine CDK tests fixed, please review again. |
@mludvig Could you revert file name modifications (test_conversation_schema.py and test_bot_usecase.py) ? This is because this change affects entire naming rule of this project. |
This reverts commit 47193d0.
Oops, sorry @statefb, I didn't mean to include it in the push. I'm unable to run pytest without it though, it keeps failing with name collisions. But that's a debate for another day. |
|
||
// Bedrock configuration | ||
bedrockRegion: z.string().default("us-east-1"), | ||
enableBedrockGlobalInference: z.boolean().default(true), |
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.
Could you add this option into cdk.json as well?
In some organisations the SCP may restrict global inference profiles while allowing the regional ones (e.g. US businesses may allow cross region inference in us-- but not global inference outside of the US). This patch adds support for
enableBedrockGlobalInference
that works the same as the existingenableBedrockCrossRegionInference
. By default it's enabled so no change for the existing users.By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.