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.

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

Added eu-west-1 registry #12

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jverstraaten
Copy link

Added the eu-west-1 ECR registry so it's possible to run sm-docker with eu-west-1 as AWS_DEFAULT_REGION

Issue #, if available:

Description of changes:
Added the eu-west-1 registry for sagemaker studio images so it's possible to use sm-docker in ireland region

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

Added the eu-west-1 ECR registry so it's possible to run sm-docker with eu-west-1 as AWS_DEFAULT_REGION
Copy link
Contributor

@jaipreet-s jaipreet-s left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution!

I checked at [1] and the repository ID for eu-west-1 is 763104351884. Where is this registry ID coming from?

[1] https://github.com/aws/deep-learning-containers/blob/master/available_images.md

@jverstraaten
Copy link
Author

We were getting these from the AWS Sagemaker docs: https://docs.aws.amazon.com/sagemaker/latest/dg/sagemaker-algo-docker-registry-paths.html. I looks like they changed it to have a separate account per region and per algorithm. I guess a second PR should be made to handle these changes in a more complete fashion but if we can merge this at least I can continue for now

@jaipreet-s
Copy link
Contributor

jaipreet-s commented Apr 19, 2021

Thanks for the details. I recommend adding all the regions' account IDs for at-least the algo in question. Otherwise, it's a confusing user experience where the tool only works in one specific region.

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.

None yet

2 participants