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.

Sign up for GitHub

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

Jump to bottom

Feature/hide not enabled models #520

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft

Feature/hide not enabled models #520

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
52ff527
Add a list of bedrock enabled foundation models to show only these on…
eldimi Jul 17, 2024
7c4e899
Merge branch 'aws-samples:main' into feature/hide_not_enabled_models
eldimi Jul 25, 2024
b509770
Merge branch 'aws-samples:main' into feature/hide_not_enabled_models
eldimi Jul 30, 2024
1bc0e75
Add a list of bedrock enabled foundation models to show only these i …
eldimi Jul 30, 2024
0b71efb
Merge branch 'main' into feature/hide_not_enabled_models
eldimi Aug 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions lib/aws-genai-llm-chatbot-stack.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ export class AwsGenAILLMChatbotStack extends cdk.Stack {
ragEngines: ragEngines,
userPool: authentication.userPool,
modelsParameter: models.modelsParameter,
bedrockEnabledModelsParameter: models.bedrockEnabledModelsParameter,
models: models.models,
});

Expand Down
1 change: 1 addition & 0 deletions lib/chatbot-api/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export interface ChatBotApiProps {
readonly ragEngines?: RagEngines;
readonly userPool: cognito.UserPool;
readonly modelsParameter: ssm.StringParameter;
readonly bedrockEnabledModelsParameter: ssm.StringParameter;
readonly models: SageMakerModelEndpoint[];
}

Expand Down
3 changes: 3 additions & 0 deletions lib/chatbot-api/rest-api.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
readonly byUserIdIndex: string;
readonly userFeedbackBucket: s3.Bucket;
readonly modelsParameter: ssm.StringParameter;
readonly bedrockEnabledModelsParameter: ssm.StringParameter;
readonly models: SageMakerModelEndpoint[];
readonly api: appsync.GraphqlApi;
}
Expand Down Expand Up @@ -59,6 +60,7 @@
...props.shared.defaultEnvironmentVariables,
CONFIG_PARAMETER_NAME: props.shared.configParameter.parameterName,
MODELS_PARAMETER_NAME: props.modelsParameter.parameterName,
BEDROCK_ENABLED_MODELS_PARAMETER_NAME: props.bedrockEnabledModelsParameter.parameterName,

Check failure on line 63 in lib/chatbot-api/rest-api.ts

View workflow job for this annotation

GitHub Actions / build-cdk 

Insert `⏎···········`
X_ORIGIN_VERIFY_SECRET_ARN:
props.shared.xOriginVerifySecret.secretArn,
API_KEYS_SECRETS_ARN: props.shared.apiKeysSecret.secretArn,
Expand Down Expand Up @@ -290,6 +292,7 @@
props.shared.apiKeysSecret.grantRead(apiHandler);
props.shared.configParameter.grantRead(apiHandler);
props.modelsParameter.grantRead(apiHandler);
props.bedrockEnabledModelsParameter.grantRead(apiHandler);
props.sessionsTable.grantReadWriteData(apiHandler);
props.userFeedbackBucket.grantReadWrite(apiHandler);
props.ragEngines?.uploadBucket.grantReadWrite(apiHandler);
Expand Down
9 changes: 9 additions & 0 deletions lib/models/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,13 @@
export class Models extends Construct {
public readonly models: SageMakerModelEndpoint[];
public readonly modelsParameter: ssm.StringParameter;
public readonly bedrockEnabledModelsParameter: ssm.StringParameter;

constructor(scope: Construct, id: string, props: ModelsProps) {
super(scope, id);

const models: SageMakerModelEndpoint[] = [];
const bedrockEnabledModels: string[] = ['anthropic.claude-3-haiku-20240307-v1:0', 'anthropic.claude-3-sonnet-20240229-v1:0'];

Check failure on line 37 in lib/models/index.ts

View workflow job for this annotation

GitHub Actions / build-cdk 

Replace `'anthropic.claude-3-haiku-20240307-v1:0',·'anthropic.claude-3-sonnet-20240229-v1:0'` with `⏎······"anthropic.claude-3-haiku-20240307-v1:0",⏎······"anthropic.claude-3-sonnet-20240229-v1:0",⏎····`
Copy link
Collaborator

Choose a reason for hiding this comment

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

This value should come from the config.json file. You need to add a new property to the SystemConfig interface to store these values and add a multiselect option to the magic config to let the user choose which models to enable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should we add a free text with comma seperated model ids or a multi-selected to select from all the available foundation models ? I would vote for free text, since it doesn't need maintance - adding new models - and a bit more user-friendly - user will type instead of scrolling through the models to select the one's needed.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I you just want to allow-list few models, a text field could work fine, but you would still need to validate the modelIds. I think a multiselect is more user friendly and you can always populate the values dynamically by querying the bedrock API. Anyway, to add new models we also need to add them to the rest of the code.


let hfTokenSecret: secretsmanager.Secret | undefined;
if (props.config.llms.huggingfaceApiSecretArn) {
Expand Down Expand Up @@ -386,8 +388,15 @@
),
});

const bedrockEnabledModelsParameter = new ssm.StringParameter(this, "BedrockEnabledModelsParameter", {

Check failure on line 391 in lib/models/index.ts

View workflow job for this annotation

GitHub Actions / build-cdk 

Replace `this,·"BedrockEnabledModelsParameter",` with `⏎······this,⏎······"BedrockEnabledModelsParameter",⏎·····`
stringValue: JSON.stringify(

Check failure on line 392 in lib/models/index.ts

View workflow job for this annotation

GitHub Actions / build-cdk 

Replace `······stringValue:·JSON.stringify(⏎········bedrockEnabledModels` with `········stringValue:·JSON.stringify(bedrockEnabledModels),`
bedrockEnabledModels
),

Check failure on line 394 in lib/models/index.ts

View workflow job for this annotation

GitHub Actions / build-cdk 

Replace `),` with `}`
});

Check failure on line 395 in lib/models/index.ts

View workflow job for this annotation

GitHub Actions / build-cdk 

Delete `}`

this.models = models;
this.modelsParameter = modelsParameter;
this.bedrockEnabledModelsParameter = bedrockEnabledModelsParameter;

if (models.length > 0 && props.config.llms?.sagemakerSchedule?.enabled) {
const schedulerRole: iam.Role = new iam.Role(this, "SchedulerRole", {
Expand Down
5 changes: 4 additions & 1 deletion lib/shared/layers/python-sdk/python/genai_core/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,11 +83,14 @@ def list_bedrock_models():
byInferenceType=genai_core.types.InferenceType.ON_DEMAND.value,
byOutputModality=genai_core.types.Modality.TEXT.value,
)

enabledModels = genai_core.parameters.get_enabled_bedrock_models()

bedrock_models = [
m
for m in response.get("modelSummaries", [])
if m.get("modelLifecycle", {}).get("status")
== genai_core.types.ModelStatus.ACTIVE.value
== genai_core.types.ModelStatus.ACTIVE.value and m.get("modelId") in enabledModels
]

models = [
Expand Down
4 changes: 4 additions & 0 deletions lib/shared/layers/python-sdk/python/genai_core/parameters.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
API_KEYS_SECRETS_ARN = os.environ.get("API_KEYS_SECRETS_ARN")
CONFIG_PARAMETER_NAME = os.environ.get("CONFIG_PARAMETER_NAME")
MODELS_PARAMETER_NAME = os.environ.get("MODELS_PARAMETER_NAME")
BEDROCK_ENABLED_MODELS_PARAMETER_NAME = os.environ.get("BEDROCK_ENABLED_MODELS_PARAMETER_NAME")


def get_external_api_key(name: str):
Expand Down Expand Up @@ -32,3 +33,6 @@ def get_config():

def get_sagemaker_models():
return parameters.get_parameter(MODELS_PARAMETER_NAME, transform="json", max_age=30)

def get_enabled_bedrock_models():
return parameters.get_parameter(BEDROCK_ENABLED_MODELS_PARAMETER_NAME, transform="json", max_age=30)
Loading