change: use image_uris.retrieve() for ModelMonitor default image #1765
Conversation
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
src/sagemaker/image_uris.py
Outdated
| @@ -22,6 +22,7 @@ | |||
| logger = logging.getLogger(__name__) | |||
|
|
|||
| ECR_URI_TEMPLATE = "{registry}.dkr.{hostname}/{repository}:{tag}" | |||
| MONITOR_URI_TEMPLATE = "{registry}.dkr.{hostname}/{repository}" | |||
There was a problem hiding this comment.
I'd prefer the code in this file be as framework/algorithm-agnostic as possible. so let's remove the tag part from the template, and then figure out how to structure the JSON instead?
this is a bit tough since I hadn't even considered that there might be URIs without tags when I started this...you could use "" for the version? or add some logic to handle the case where "versions" isn't a valid key.
There was a problem hiding this comment.
I updated the model-monitor.json file as well as the retrieve() logic to let model monitor use the generic template.
We could update the "versioins" later when model-monitor images with tags are released.
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
| "versions": { | ||
| "": { | ||
| "registries": { | ||
| "eu-north-1": "895015795356", |
There was a problem hiding this comment.
nitpicky nitpick: alphabetize the regions
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Issue #, if available:
#1464
Description of changes:
Use image_uris.retrieve() for ModelMonitor default image uri
Testing done:
unit tests
Merge Checklist
Put an
xin the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your pull request.General
Tests
unique_name_from_baseto create resource names in integ tests (if appropriate)By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.