-
Notifications
You must be signed in to change notification settings - Fork 17.8k
Skip legacy models when selecting a Bedrock inference profile #72521
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: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,6 +26,11 @@ | |
| from airflow.decorators import task # type: ignore[attr-defined, no-redef] | ||
|
|
||
|
|
||
| def _foundation_model_id(model_arn: str) -> str: | ||
| """Return the model ID part of a foundation model ARN, which is identical in every region.""" | ||
| return model_arn.rpartition("/")[2] | ||
|
|
||
|
|
||
| @task | ||
| def get_text_inference_profile_arn() -> str: | ||
| """ | ||
|
|
@@ -37,11 +42,27 @@ def get_text_inference_profile_arn() -> str: | |
| from airflow.providers.amazon.aws.hooks.bedrock import BedrockHook | ||
|
|
||
| client = BedrockHook().conn | ||
|
|
||
| # Bedrock rejects a model its provider marked as legacy, so a legacy model can not be relied on here. | ||
| # The inference profile summaries do not carry the lifecycle status, only the foundation models a | ||
| # profile resolves to do. | ||
| legacy_model_ids = { | ||
| model["modelId"] | ||
| for model in client.list_foundation_models()["modelSummaries"] | ||
| if model.get("modelLifecycle", {}).get("status") == "LEGACY" | ||
| } | ||
| log.info("Legacy model IDs: %s", sorted(legacy_model_ids)) | ||
|
|
||
| profiles = client.list_inference_profiles(typeEquals="SYSTEM_DEFINED")["inferenceProfileSummaries"] | ||
| arns = [ | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since the pick among non-legacy candidates still follows list_inference_profiles order, the model under test can change between runs. Would it make sense to add a deterministic tiebreak, e.g. sort the candidates and pick the oldest non-legacy Sonnet, so runs are reproducible and we don't land on a brand-new release before batch inference or RAG support it? |
||
| profile["inferenceProfileArn"] | ||
| for profile in profiles | ||
| if profile.get("status") == "ACTIVE" and profile["inferenceProfileId"].startswith("global.anthropic.") | ||
| if profile.get("status") == "ACTIVE" | ||
| and profile["inferenceProfileId"].startswith("global.anthropic.") | ||
| and not any( | ||
| _foundation_model_id(model["modelArn"]) in legacy_model_ids | ||
| for model in profile.get("models", []) | ||
| ) | ||
| ] | ||
| log.info("Valid text inference profile ARNs: %s", arns) | ||
|
|
||
|
|
@@ -50,4 +71,4 @@ def get_text_inference_profile_arn() -> str: | |
| if "sonnet" in arn: | ||
| log.info("Selected inference profile ARN: %s", arn) | ||
| return arn | ||
| raise RuntimeError("No valid inference profiles found") | ||
| raise RuntimeError(f"No valid inference profiles found. Non legacy candidates were: {arns}") | ||
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.
Should we filter for just
ACTIVEso that if there's a new state likeDEPRECATEDor something else, this would still keep working?