fix: change AMI ids in tests to be dynamic based on regions - #1004

Merged
chuyang-deng merged 10 commits into
aws:masterfrom
caxiaohu:fix_region_issue
Aug 28, 2019
Merged

fix: change AMI ids in tests to be dynamic based on regions#1004
chuyang-deng merged 10 commits into
aws:masterfrom
caxiaohu:fix_region_issue

Conversation

@caxiaohu

@caxiaohucaxiaohu commented Aug 27, 2019

Copy link
Copy Markdown
Contributor

Issue #, if available:

Description of changes:

Merge Checklist

Put an x in 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.

  • I have read the CONTRIBUTING doc
  • I used the commit message format described in CONTRIBUTING
  • I have added tests that prove my fix is effective or that my feature works (if appropriate)
  • I have updated any necessary documentation (if appropriate)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository


echo "Mounting EFS File Systems"
sudo yum install -y amazon-efs-utils.noarch 0:1.10-1.amzn2
sudo yum install -y amazon-efs-utils

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

do different regions have different versions released?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

The list for the AMI ids are Amazon Linux AMI 2018, while the old hard-coded one is Amazon Linux2 AMI.
The new list needs amazon-efs-utils.noarch 0:1.10-1.amzn1 version. I just don't want specify specific version in case some of them need amazon-efs-utils.noarch 0:1.10-1.amzn2 version.
But it will automatically select the correct version if i don't specify

ROLE_NAME = "SageMakerRole"
REGION = "us-west-2"
EC2_INSTANCE_TYPE = "t2.micro"
AMI_ID = "ami-082b5a644766e0e6f"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think you can remove this

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Forgot to remove it

KEY_PATH = os.path.join(tempfile.gettempdir(), FILE_NAME)
STORAGE_CAPACITY_IN_BYTES = 3600

AWSRegionArch2AMI = {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

rename this to REGION_TO_AMI_MAP - we still want to honor Python capitalization/naming convention

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Sure

"cn-north-1": "ami-0a4eaf6c4454eda75",
"cn-northwest-1": "ami-6b6a7d09",
"us-gov-west-1": "ami-906cf0f1",
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@caxiaohu@laurenyu Is there any way we could make this dynamic based on the AMI name or type? Since we only need a basic AL AMI?
i.e.: Some dynamic logic that searches for an AMI by name and uses whatever ID is appropriate in that region. Eliminating the need for a map that we need to maintain.

I'm concerned that we'll need to add to this list for all future regions. This is likely another time-bomb that'll trigger a page on region expansion (or the dev will catch it during region expansion).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

good catch. I'd forgotten that I found this yesterday; I think it should do what we want: https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/finding-an-ami.html#finding-quick-start-ami

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Done!

@caxiaohucaxiaohuAug 27, 2019

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

The AMI ids search results:

RegionAMI id
us-west-1ami-0fcdcdb074d2bac5f
us-west-2ami-0f2176987ee50226e
us-east-1ami-035b3c7efe6d061d5
us-east-2ami-02f706d959cedf892
eu-west-1ami-0862aabda3fb488b5
eu-west-2ami-0bdfa1adc3878cd23
eu-west-3ami-05b93cd5a1b552734
eu-central-1ami-026d3b3672c6e7b66
ap-northeast-1ami-04b2d1589ab1d972c
ap-northeast-2ami-0be3e6f84d3b968cd
ap-northeast-3ami-0639e0e4c18187b30
ap-southeast-1ami-0fb6b6f9e81056553
ap-southeast-2ami-075caa3491def750b
ap-south-1ami-0b99c7725b9484f9e
ca-central-1ami-0a67d15f2858e33cb
sa-east-1ami-0bb96001cf2299257
cn-north-1AWS was not able to validate the provided access credentials
cn-northwest-1AWS was not able to validate the provided access credentials
us-gov-west-1AWS was not able to validate the provided access credentials

All of them are Amazon Linux AMI 2018.03.0

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

return fs_resources


def _dynamic_ami_id(sagemaker_session):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

let's rename this to _ami_id_for_region

def _dynamic_ami_id(sagemaker_session):
ec2_client = sagemaker_session.boto_session.client("ec2")
filters = [
{"Name": "name", "Values": ["amzn-ami-hvm-????.??.?.????????-x86_64-gp2"]},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

let's make this a constant. that'll both make it easier to find if it needs to be changed later for whatever reason and help describe what the string is.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Sure

image_details = sorted(response["Images"], key=itemgetter("CreationDate"), reverse=True)
if len(image_details) > 0:
ami_id = image_details[0]["ImageId"]
return ami_id

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: you can combine ll. 126-127

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Sure

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

STORAGE_CAPACITY_IN_BYTES = 3600

AMI_FILTERS = [
{"Name": "name", "Values": ["amzn-ami-hvm-????.??.?.????????-x86_64-gp2"]},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

sorry for the confusion. I meant specifically "amzn-ami-hvm-????.??.?.????????-x86_64-gp2" - it's not really obvious what this string means

@caxiaohucaxiaohuAug 27, 2019

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I see what you mean now

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@laurenyulaurenyu changed the title fix: changed AMI ids to be dynamic based on regionsfix: change AMI ids in tests to be dynamic based on regionsAug 27, 2019
@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@chuyang-deng
chuyang-deng merged commit f233dc0 into aws:masterAug 28, 2019
@chuyang-dengchuyang-deng mentioned this pull request Aug 28, 2019
4 tasks
shenlongtang added a commit to shenlongtang/sagemaker-python-sdk that referenced this pull request Oct 3, 2023
zuoyuanh added a commit that referenced this pull request Oct 5, 2023
* feature: method to build pipeline parameters from existing execution … (#951)
* feature: method to build pipeline parameters from existing execution with optional value overrides
* fix style check
* assert error message in unit test
* feature: allow opt out from referencing latest execution in the selec… (#1004)
* fix: Update pipeline.py and selective_execution_config.py with small fixes (#1099)
---------
Co-authored-by: stacicho <stacicho@amazon.com>
Co-authored-by: Zuoyuan Huang <david.hzy97@gmail.com>
Evan-W-ang added a commit to Evan-W-ang/sagemaker-python-sdk that referenced this pull request Jun 8, 2026
* fix: change Amazon AMI ids to be dynamic based on regions using searching Amazon Linux AMI
Evan-W-ang added a commit to Evan-W-ang/sagemaker-python-sdk that referenced this pull request Jun 8, 2026
* feature: method to build pipeline parameters from existing execution … (aws#951)
* feature: method to build pipeline parameters from existing execution with optional value overrides
* fix style check
* assert error message in unit test
* feature: allow opt out from referencing latest execution in the selec… (aws#1004)
* fix: Update pipeline.py and selective_execution_config.py with small fixes (aws#1099)
---------
Co-authored-by: stacicho <stacicho@amazon.com>
Co-authored-by: Zuoyuan Huang <david.hzy97@gmail.com>
Sign up for freeto 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.

5 participants

@caxiaohu@sagemaker-bot@laurenyu@chuyang-deng@knakad
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

fix: change AMI ids in tests to be dynamic based on regions - #1004

Merged
chuyang-deng merged 10 commits into
aws:masterfrom
caxiaohu:fix_region_issue
Aug 28, 2019
Merged

fix: change AMI ids in tests to be dynamic based on regions#1004
chuyang-deng merged 10 commits into
aws:masterfrom
caxiaohu:fix_region_issue

Conversation

@caxiaohu

@caxiaohucaxiaohu commented Aug 27, 2019

Copy link
Copy Markdown
Contributor

Issue #, if available:

Description of changes:

Merge Checklist

Put an x in 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.

  • I have read the CONTRIBUTING doc
  • I used the commit message format described in CONTRIBUTING
  • I have added tests that prove my fix is effective or that my feature works (if appropriate)
  • I have updated any necessary documentation (if appropriate)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository


echo "Mounting EFS File Systems"
sudo yum install -y amazon-efs-utils.noarch 0:1.10-1.amzn2
sudo yum install -y amazon-efs-utils

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

do different regions have different versions released?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

The list for the AMI ids are Amazon Linux AMI 2018, while the old hard-coded one is Amazon Linux2 AMI.
The new list needs amazon-efs-utils.noarch 0:1.10-1.amzn1 version. I just don't want specify specific version in case some of them need amazon-efs-utils.noarch 0:1.10-1.amzn2 version.
But it will automatically select the correct version if i don't specify

ROLE_NAME = "SageMakerRole"
REGION = "us-west-2"
EC2_INSTANCE_TYPE = "t2.micro"
AMI_ID = "ami-082b5a644766e0e6f"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think you can remove this

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Forgot to remove it

KEY_PATH = os.path.join(tempfile.gettempdir(), FILE_NAME)
STORAGE_CAPACITY_IN_BYTES = 3600

AWSRegionArch2AMI = {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

rename this to REGION_TO_AMI_MAP - we still want to honor Python capitalization/naming convention

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Sure

"cn-north-1": "ami-0a4eaf6c4454eda75",
"cn-northwest-1": "ami-6b6a7d09",
"us-gov-west-1": "ami-906cf0f1",
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@caxiaohu@laurenyu Is there any way we could make this dynamic based on the AMI name or type? Since we only need a basic AL AMI?
i.e.: Some dynamic logic that searches for an AMI by name and uses whatever ID is appropriate in that region. Eliminating the need for a map that we need to maintain.

I'm concerned that we'll need to add to this list for all future regions. This is likely another time-bomb that'll trigger a page on region expansion (or the dev will catch it during region expansion).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

good catch. I'd forgotten that I found this yesterday; I think it should do what we want: https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/finding-an-ami.html#finding-quick-start-ami

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Done!

@caxiaohucaxiaohuAug 27, 2019

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

The AMI ids search results:

RegionAMI id
us-west-1ami-0fcdcdb074d2bac5f
us-west-2ami-0f2176987ee50226e
us-east-1ami-035b3c7efe6d061d5
us-east-2ami-02f706d959cedf892
eu-west-1ami-0862aabda3fb488b5
eu-west-2ami-0bdfa1adc3878cd23
eu-west-3ami-05b93cd5a1b552734
eu-central-1ami-026d3b3672c6e7b66
ap-northeast-1ami-04b2d1589ab1d972c
ap-northeast-2ami-0be3e6f84d3b968cd
ap-northeast-3ami-0639e0e4c18187b30
ap-southeast-1ami-0fb6b6f9e81056553
ap-southeast-2ami-075caa3491def750b
ap-south-1ami-0b99c7725b9484f9e
ca-central-1ami-0a67d15f2858e33cb
sa-east-1ami-0bb96001cf2299257
cn-north-1AWS was not able to validate the provided access credentials
cn-northwest-1AWS was not able to validate the provided access credentials
us-gov-west-1AWS was not able to validate the provided access credentials

All of them are Amazon Linux AMI 2018.03.0

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

return fs_resources


def _dynamic_ami_id(sagemaker_session):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

let's rename this to _ami_id_for_region

def _dynamic_ami_id(sagemaker_session):
ec2_client = sagemaker_session.boto_session.client("ec2")
filters = [
{"Name": "name", "Values": ["amzn-ami-hvm-????.??.?.????????-x86_64-gp2"]},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

let's make this a constant. that'll both make it easier to find if it needs to be changed later for whatever reason and help describe what the string is.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Sure

image_details = sorted(response["Images"], key=itemgetter("CreationDate"), reverse=True)
if len(image_details) > 0:
ami_id = image_details[0]["ImageId"]
return ami_id

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: you can combine ll. 126-127

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Sure

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

STORAGE_CAPACITY_IN_BYTES = 3600

AMI_FILTERS = [
{"Name": "name", "Values": ["amzn-ami-hvm-????.??.?.????????-x86_64-gp2"]},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

sorry for the confusion. I meant specifically "amzn-ami-hvm-????.??.?.????????-x86_64-gp2" - it's not really obvious what this string means

@caxiaohucaxiaohuAug 27, 2019

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I see what you mean now

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@laurenyulaurenyu changed the title fix: changed AMI ids to be dynamic based on regionsfix: change AMI ids in tests to be dynamic based on regionsAug 27, 2019
@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@chuyang-deng
chuyang-deng merged commit f233dc0 into aws:masterAug 28, 2019
@chuyang-dengchuyang-deng mentioned this pull request Aug 28, 2019
4 tasks
shenlongtang added a commit to shenlongtang/sagemaker-python-sdk that referenced this pull request Oct 3, 2023
zuoyuanh added a commit that referenced this pull request Oct 5, 2023
* feature: method to build pipeline parameters from existing execution … (#951)
* feature: method to build pipeline parameters from existing execution with optional value overrides
* fix style check
* assert error message in unit test
* feature: allow opt out from referencing latest execution in the selec… (#1004)
* fix: Update pipeline.py and selective_execution_config.py with small fixes (#1099)
---------
Co-authored-by: stacicho <stacicho@amazon.com>
Co-authored-by: Zuoyuan Huang <david.hzy97@gmail.com>
Evan-W-ang added a commit to Evan-W-ang/sagemaker-python-sdk that referenced this pull request Jun 8, 2026
* fix: change Amazon AMI ids to be dynamic based on regions using searching Amazon Linux AMI
Evan-W-ang added a commit to Evan-W-ang/sagemaker-python-sdk that referenced this pull request Jun 8, 2026
* feature: method to build pipeline parameters from existing execution … (aws#951)
* feature: method to build pipeline parameters from existing execution with optional value overrides
* fix style check
* assert error message in unit test
* feature: allow opt out from referencing latest execution in the selec… (aws#1004)
* fix: Update pipeline.py and selective_execution_config.py with small fixes (aws#1099)
---------
Co-authored-by: stacicho <stacicho@amazon.com>
Co-authored-by: Zuoyuan Huang <david.hzy97@gmail.com>
Sign up for freeto 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.

5 participants

@caxiaohu@sagemaker-bot@laurenyu@chuyang-deng@knakad
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix: change AMI ids in tests to be dynamic based on regions - #1004

Merged
chuyang-deng merged 10 commits into
aws:masterfrom
caxiaohu:fix_region_issue
Aug 28, 2019
Merged

fix: change AMI ids in tests to be dynamic based on regions#1004
chuyang-deng merged 10 commits into
aws:masterfrom
caxiaohu:fix_region_issue

Conversation

@caxiaohu

@caxiaohucaxiaohu commented Aug 27, 2019

Copy link
Copy Markdown
Contributor

Issue #, if available:

Description of changes:

Merge Checklist

Put an x in 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.

  • I have read the CONTRIBUTING doc
  • I used the commit message format described in CONTRIBUTING
  • I have added tests that prove my fix is effective or that my feature works (if appropriate)
  • I have updated any necessary documentation (if appropriate)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository


echo "Mounting EFS File Systems"
sudo yum install -y amazon-efs-utils.noarch 0:1.10-1.amzn2
sudo yum install -y amazon-efs-utils

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

do different regions have different versions released?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

The list for the AMI ids are Amazon Linux AMI 2018, while the old hard-coded one is Amazon Linux2 AMI.
The new list needs amazon-efs-utils.noarch 0:1.10-1.amzn1 version. I just don't want specify specific version in case some of them need amazon-efs-utils.noarch 0:1.10-1.amzn2 version.
But it will automatically select the correct version if i don't specify

ROLE_NAME = "SageMakerRole"
REGION = "us-west-2"
EC2_INSTANCE_TYPE = "t2.micro"
AMI_ID = "ami-082b5a644766e0e6f"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think you can remove this

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Forgot to remove it

KEY_PATH = os.path.join(tempfile.gettempdir(), FILE_NAME)
STORAGE_CAPACITY_IN_BYTES = 3600

AWSRegionArch2AMI = {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

rename this to REGION_TO_AMI_MAP - we still want to honor Python capitalization/naming convention

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Sure

"cn-north-1": "ami-0a4eaf6c4454eda75",
"cn-northwest-1": "ami-6b6a7d09",
"us-gov-west-1": "ami-906cf0f1",
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@caxiaohu@laurenyu Is there any way we could make this dynamic based on the AMI name or type? Since we only need a basic AL AMI?
i.e.: Some dynamic logic that searches for an AMI by name and uses whatever ID is appropriate in that region. Eliminating the need for a map that we need to maintain.

I'm concerned that we'll need to add to this list for all future regions. This is likely another time-bomb that'll trigger a page on region expansion (or the dev will catch it during region expansion).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

good catch. I'd forgotten that I found this yesterday; I think it should do what we want: https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/finding-an-ami.html#finding-quick-start-ami

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Done!

@caxiaohucaxiaohuAug 27, 2019

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

The AMI ids search results:

RegionAMI id
us-west-1ami-0fcdcdb074d2bac5f
us-west-2ami-0f2176987ee50226e
us-east-1ami-035b3c7efe6d061d5
us-east-2ami-02f706d959cedf892
eu-west-1ami-0862aabda3fb488b5
eu-west-2ami-0bdfa1adc3878cd23
eu-west-3ami-05b93cd5a1b552734
eu-central-1ami-026d3b3672c6e7b66
ap-northeast-1ami-04b2d1589ab1d972c
ap-northeast-2ami-0be3e6f84d3b968cd
ap-northeast-3ami-0639e0e4c18187b30
ap-southeast-1ami-0fb6b6f9e81056553
ap-southeast-2ami-075caa3491def750b
ap-south-1ami-0b99c7725b9484f9e
ca-central-1ami-0a67d15f2858e33cb
sa-east-1ami-0bb96001cf2299257
cn-north-1AWS was not able to validate the provided access credentials
cn-northwest-1AWS was not able to validate the provided access credentials
us-gov-west-1AWS was not able to validate the provided access credentials

All of them are Amazon Linux AMI 2018.03.0

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

return fs_resources


def _dynamic_ami_id(sagemaker_session):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

let's rename this to _ami_id_for_region

def _dynamic_ami_id(sagemaker_session):
ec2_client = sagemaker_session.boto_session.client("ec2")
filters = [
{"Name": "name", "Values": ["amzn-ami-hvm-????.??.?.????????-x86_64-gp2"]},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

let's make this a constant. that'll both make it easier to find if it needs to be changed later for whatever reason and help describe what the string is.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Sure

image_details = sorted(response["Images"], key=itemgetter("CreationDate"), reverse=True)
if len(image_details) > 0:
ami_id = image_details[0]["ImageId"]
return ami_id

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: you can combine ll. 126-127

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Sure

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

STORAGE_CAPACITY_IN_BYTES = 3600

AMI_FILTERS = [
{"Name": "name", "Values": ["amzn-ami-hvm-????.??.?.????????-x86_64-gp2"]},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

sorry for the confusion. I meant specifically "amzn-ami-hvm-????.??.?.????????-x86_64-gp2" - it's not really obvious what this string means

@caxiaohucaxiaohuAug 27, 2019

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I see what you mean now

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@laurenyulaurenyu changed the title fix: changed AMI ids to be dynamic based on regionsfix: change AMI ids in tests to be dynamic based on regionsAug 27, 2019
@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@chuyang-deng
chuyang-deng merged commit f233dc0 into aws:masterAug 28, 2019
@chuyang-dengchuyang-deng mentioned this pull request Aug 28, 2019
4 tasks
shenlongtang added a commit to shenlongtang/sagemaker-python-sdk that referenced this pull request Oct 3, 2023
zuoyuanh added a commit that referenced this pull request Oct 5, 2023
* feature: method to build pipeline parameters from existing execution … (#951)
* feature: method to build pipeline parameters from existing execution with optional value overrides
* fix style check
* assert error message in unit test
* feature: allow opt out from referencing latest execution in the selec… (#1004)
* fix: Update pipeline.py and selective_execution_config.py with small fixes (#1099)
---------
Co-authored-by: stacicho <stacicho@amazon.com>
Co-authored-by: Zuoyuan Huang <david.hzy97@gmail.com>
Evan-W-ang added a commit to Evan-W-ang/sagemaker-python-sdk that referenced this pull request Jun 8, 2026
* fix: change Amazon AMI ids to be dynamic based on regions using searching Amazon Linux AMI
Evan-W-ang added a commit to Evan-W-ang/sagemaker-python-sdk that referenced this pull request Jun 8, 2026
* feature: method to build pipeline parameters from existing execution … (aws#951)
* feature: method to build pipeline parameters from existing execution with optional value overrides
* fix style check
* assert error message in unit test
* feature: allow opt out from referencing latest execution in the selec… (aws#1004)
* fix: Update pipeline.py and selective_execution_config.py with small fixes (aws#1099)
---------
Co-authored-by: stacicho <stacicho@amazon.com>
Co-authored-by: Zuoyuan Huang <david.hzy97@gmail.com>
Sign up for freeto 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.

5 participants

@caxiaohu@sagemaker-bot@laurenyu@chuyang-deng@knakad
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix: change AMI ids in tests to be dynamic based on regions - #1004

Merged
chuyang-deng merged 10 commits into
aws:masterfrom
caxiaohu:fix_region_issue
Aug 28, 2019
Merged

fix: change AMI ids in tests to be dynamic based on regions#1004
chuyang-deng merged 10 commits into
aws:masterfrom
caxiaohu:fix_region_issue

Conversation

@caxiaohu

@caxiaohucaxiaohu commented Aug 27, 2019

Copy link
Copy Markdown
Contributor

Issue #, if available:

Description of changes:

Merge Checklist

Put an x in 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.

  • I have read the CONTRIBUTING doc
  • I used the commit message format described in CONTRIBUTING
  • I have added tests that prove my fix is effective or that my feature works (if appropriate)
  • I have updated any necessary documentation (if appropriate)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository


echo "Mounting EFS File Systems"
sudo yum install -y amazon-efs-utils.noarch 0:1.10-1.amzn2
sudo yum install -y amazon-efs-utils

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

do different regions have different versions released?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

The list for the AMI ids are Amazon Linux AMI 2018, while the old hard-coded one is Amazon Linux2 AMI.
The new list needs amazon-efs-utils.noarch 0:1.10-1.amzn1 version. I just don't want specify specific version in case some of them need amazon-efs-utils.noarch 0:1.10-1.amzn2 version.
But it will automatically select the correct version if i don't specify

ROLE_NAME = "SageMakerRole"
REGION = "us-west-2"
EC2_INSTANCE_TYPE = "t2.micro"
AMI_ID = "ami-082b5a644766e0e6f"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think you can remove this

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Forgot to remove it

KEY_PATH = os.path.join(tempfile.gettempdir(), FILE_NAME)
STORAGE_CAPACITY_IN_BYTES = 3600

AWSRegionArch2AMI = {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

rename this to REGION_TO_AMI_MAP - we still want to honor Python capitalization/naming convention

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Sure

"cn-north-1": "ami-0a4eaf6c4454eda75",
"cn-northwest-1": "ami-6b6a7d09",
"us-gov-west-1": "ami-906cf0f1",
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@caxiaohu@laurenyu Is there any way we could make this dynamic based on the AMI name or type? Since we only need a basic AL AMI?
i.e.: Some dynamic logic that searches for an AMI by name and uses whatever ID is appropriate in that region. Eliminating the need for a map that we need to maintain.

I'm concerned that we'll need to add to this list for all future regions. This is likely another time-bomb that'll trigger a page on region expansion (or the dev will catch it during region expansion).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

good catch. I'd forgotten that I found this yesterday; I think it should do what we want: https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/finding-an-ami.html#finding-quick-start-ami

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Done!

@caxiaohucaxiaohuAug 27, 2019

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

The AMI ids search results:

RegionAMI id
us-west-1ami-0fcdcdb074d2bac5f
us-west-2ami-0f2176987ee50226e
us-east-1ami-035b3c7efe6d061d5
us-east-2ami-02f706d959cedf892
eu-west-1ami-0862aabda3fb488b5
eu-west-2ami-0bdfa1adc3878cd23
eu-west-3ami-05b93cd5a1b552734
eu-central-1ami-026d3b3672c6e7b66
ap-northeast-1ami-04b2d1589ab1d972c
ap-northeast-2ami-0be3e6f84d3b968cd
ap-northeast-3ami-0639e0e4c18187b30
ap-southeast-1ami-0fb6b6f9e81056553
ap-southeast-2ami-075caa3491def750b
ap-south-1ami-0b99c7725b9484f9e
ca-central-1ami-0a67d15f2858e33cb
sa-east-1ami-0bb96001cf2299257
cn-north-1AWS was not able to validate the provided access credentials
cn-northwest-1AWS was not able to validate the provided access credentials
us-gov-west-1AWS was not able to validate the provided access credentials

All of them are Amazon Linux AMI 2018.03.0

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

return fs_resources


def _dynamic_ami_id(sagemaker_session):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

let's rename this to _ami_id_for_region

def _dynamic_ami_id(sagemaker_session):
ec2_client = sagemaker_session.boto_session.client("ec2")
filters = [
{"Name": "name", "Values": ["amzn-ami-hvm-????.??.?.????????-x86_64-gp2"]},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

let's make this a constant. that'll both make it easier to find if it needs to be changed later for whatever reason and help describe what the string is.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Sure

image_details = sorted(response["Images"], key=itemgetter("CreationDate"), reverse=True)
if len(image_details) > 0:
ami_id = image_details[0]["ImageId"]
return ami_id

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: you can combine ll. 126-127

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Sure

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

STORAGE_CAPACITY_IN_BYTES = 3600

AMI_FILTERS = [
{"Name": "name", "Values": ["amzn-ami-hvm-????.??.?.????????-x86_64-gp2"]},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

sorry for the confusion. I meant specifically "amzn-ami-hvm-????.??.?.????????-x86_64-gp2" - it's not really obvious what this string means

@caxiaohucaxiaohuAug 27, 2019

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I see what you mean now

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@laurenyulaurenyu changed the title fix: changed AMI ids to be dynamic based on regionsfix: change AMI ids in tests to be dynamic based on regionsAug 27, 2019
@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@chuyang-deng
chuyang-deng merged commit f233dc0 into aws:masterAug 28, 2019
@chuyang-dengchuyang-deng mentioned this pull request Aug 28, 2019
4 tasks
shenlongtang added a commit to shenlongtang/sagemaker-python-sdk that referenced this pull request Oct 3, 2023
zuoyuanh added a commit that referenced this pull request Oct 5, 2023
* feature: method to build pipeline parameters from existing execution … (#951)
* feature: method to build pipeline parameters from existing execution with optional value overrides
* fix style check
* assert error message in unit test
* feature: allow opt out from referencing latest execution in the selec… (#1004)
* fix: Update pipeline.py and selective_execution_config.py with small fixes (#1099)
---------
Co-authored-by: stacicho <stacicho@amazon.com>
Co-authored-by: Zuoyuan Huang <david.hzy97@gmail.com>
Evan-W-ang added a commit to Evan-W-ang/sagemaker-python-sdk that referenced this pull request Jun 8, 2026
* fix: change Amazon AMI ids to be dynamic based on regions using searching Amazon Linux AMI
Evan-W-ang added a commit to Evan-W-ang/sagemaker-python-sdk that referenced this pull request Jun 8, 2026
* feature: method to build pipeline parameters from existing execution … (aws#951)
* feature: method to build pipeline parameters from existing execution with optional value overrides
* fix style check
* assert error message in unit test
* feature: allow opt out from referencing latest execution in the selec… (aws#1004)
* fix: Update pipeline.py and selective_execution_config.py with small fixes (aws#1099)
---------
Co-authored-by: stacicho <stacicho@amazon.com>
Co-authored-by: Zuoyuan Huang <david.hzy97@gmail.com>
Sign up for freeto 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.

5 participants

@caxiaohu@sagemaker-bot@laurenyu@chuyang-deng@knakad
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

fix: change AMI ids in tests to be dynamic based on regions - #1004

Merged
chuyang-deng merged 10 commits into
aws:masterfrom
caxiaohu:fix_region_issue
Aug 28, 2019
Merged

fix: change AMI ids in tests to be dynamic based on regions#1004
chuyang-deng merged 10 commits into
aws:masterfrom
caxiaohu:fix_region_issue

Conversation

@caxiaohu

@caxiaohucaxiaohu commented Aug 27, 2019

Copy link
Copy Markdown
Contributor

Issue #, if available:

Description of changes:

Merge Checklist

Put an x in 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.

  • I have read the CONTRIBUTING doc
  • I used the commit message format described in CONTRIBUTING
  • I have added tests that prove my fix is effective or that my feature works (if appropriate)
  • I have updated any necessary documentation (if appropriate)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository


echo "Mounting EFS File Systems"
sudo yum install -y amazon-efs-utils.noarch 0:1.10-1.amzn2
sudo yum install -y amazon-efs-utils

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

do different regions have different versions released?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

The list for the AMI ids are Amazon Linux AMI 2018, while the old hard-coded one is Amazon Linux2 AMI.
The new list needs amazon-efs-utils.noarch 0:1.10-1.amzn1 version. I just don't want specify specific version in case some of them need amazon-efs-utils.noarch 0:1.10-1.amzn2 version.
But it will automatically select the correct version if i don't specify

ROLE_NAME = "SageMakerRole"
REGION = "us-west-2"
EC2_INSTANCE_TYPE = "t2.micro"
AMI_ID = "ami-082b5a644766e0e6f"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think you can remove this

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Forgot to remove it

KEY_PATH = os.path.join(tempfile.gettempdir(), FILE_NAME)
STORAGE_CAPACITY_IN_BYTES = 3600

AWSRegionArch2AMI = {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

rename this to REGION_TO_AMI_MAP - we still want to honor Python capitalization/naming convention

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Sure

"cn-north-1": "ami-0a4eaf6c4454eda75",
"cn-northwest-1": "ami-6b6a7d09",
"us-gov-west-1": "ami-906cf0f1",
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@caxiaohu@laurenyu Is there any way we could make this dynamic based on the AMI name or type? Since we only need a basic AL AMI?
i.e.: Some dynamic logic that searches for an AMI by name and uses whatever ID is appropriate in that region. Eliminating the need for a map that we need to maintain.

I'm concerned that we'll need to add to this list for all future regions. This is likely another time-bomb that'll trigger a page on region expansion (or the dev will catch it during region expansion).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

good catch. I'd forgotten that I found this yesterday; I think it should do what we want: https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/finding-an-ami.html#finding-quick-start-ami

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Done!

@caxiaohucaxiaohuAug 27, 2019

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

The AMI ids search results:

RegionAMI id
us-west-1ami-0fcdcdb074d2bac5f
us-west-2ami-0f2176987ee50226e
us-east-1ami-035b3c7efe6d061d5
us-east-2ami-02f706d959cedf892
eu-west-1ami-0862aabda3fb488b5
eu-west-2ami-0bdfa1adc3878cd23
eu-west-3ami-05b93cd5a1b552734
eu-central-1ami-026d3b3672c6e7b66
ap-northeast-1ami-04b2d1589ab1d972c
ap-northeast-2ami-0be3e6f84d3b968cd
ap-northeast-3ami-0639e0e4c18187b30
ap-southeast-1ami-0fb6b6f9e81056553
ap-southeast-2ami-075caa3491def750b
ap-south-1ami-0b99c7725b9484f9e
ca-central-1ami-0a67d15f2858e33cb
sa-east-1ami-0bb96001cf2299257
cn-north-1AWS was not able to validate the provided access credentials
cn-northwest-1AWS was not able to validate the provided access credentials
us-gov-west-1AWS was not able to validate the provided access credentials

All of them are Amazon Linux AMI 2018.03.0

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

return fs_resources


def _dynamic_ami_id(sagemaker_session):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

let's rename this to _ami_id_for_region

def _dynamic_ami_id(sagemaker_session):
ec2_client = sagemaker_session.boto_session.client("ec2")
filters = [
{"Name": "name", "Values": ["amzn-ami-hvm-????.??.?.????????-x86_64-gp2"]},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

let's make this a constant. that'll both make it easier to find if it needs to be changed later for whatever reason and help describe what the string is.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Sure

image_details = sorted(response["Images"], key=itemgetter("CreationDate"), reverse=True)
if len(image_details) > 0:
ami_id = image_details[0]["ImageId"]
return ami_id

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: you can combine ll. 126-127

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Sure

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

STORAGE_CAPACITY_IN_BYTES = 3600

AMI_FILTERS = [
{"Name": "name", "Values": ["amzn-ami-hvm-????.??.?.????????-x86_64-gp2"]},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

sorry for the confusion. I meant specifically "amzn-ami-hvm-????.??.?.????????-x86_64-gp2" - it's not really obvious what this string means

@caxiaohucaxiaohuAug 27, 2019

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I see what you mean now

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@laurenyulaurenyu changed the title fix: changed AMI ids to be dynamic based on regionsfix: change AMI ids in tests to be dynamic based on regionsAug 27, 2019
@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@chuyang-deng
chuyang-deng merged commit f233dc0 into aws:masterAug 28, 2019
@chuyang-dengchuyang-deng mentioned this pull request Aug 28, 2019
4 tasks
shenlongtang added a commit to shenlongtang/sagemaker-python-sdk that referenced this pull request Oct 3, 2023
zuoyuanh added a commit that referenced this pull request Oct 5, 2023
* feature: method to build pipeline parameters from existing execution … (#951)
* feature: method to build pipeline parameters from existing execution with optional value overrides
* fix style check
* assert error message in unit test
* feature: allow opt out from referencing latest execution in the selec… (#1004)
* fix: Update pipeline.py and selective_execution_config.py with small fixes (#1099)
---------
Co-authored-by: stacicho <stacicho@amazon.com>
Co-authored-by: Zuoyuan Huang <david.hzy97@gmail.com>
Evan-W-ang added a commit to Evan-W-ang/sagemaker-python-sdk that referenced this pull request Jun 8, 2026
* fix: change Amazon AMI ids to be dynamic based on regions using searching Amazon Linux AMI
Evan-W-ang added a commit to Evan-W-ang/sagemaker-python-sdk that referenced this pull request Jun 8, 2026
* feature: method to build pipeline parameters from existing execution … (aws#951)
* feature: method to build pipeline parameters from existing execution with optional value overrides
* fix style check
* assert error message in unit test
* feature: allow opt out from referencing latest execution in the selec… (aws#1004)
* fix: Update pipeline.py and selective_execution_config.py with small fixes (aws#1099)
---------
Co-authored-by: stacicho <stacicho@amazon.com>
Co-authored-by: Zuoyuan Huang <david.hzy97@gmail.com>
Sign up for freeto 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.

5 participants

@caxiaohu@sagemaker-bot@laurenyu@chuyang-deng@knakad
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix: change AMI ids in tests to be dynamic based on regions - #1004

Merged
chuyang-deng merged 10 commits into
aws:masterfrom
caxiaohu:fix_region_issue
Aug 28, 2019
Merged

fix: change AMI ids in tests to be dynamic based on regions#1004
chuyang-deng merged 10 commits into
aws:masterfrom
caxiaohu:fix_region_issue

Conversation

@caxiaohu

@caxiaohucaxiaohu commented Aug 27, 2019

Copy link
Copy Markdown
Contributor

Issue #, if available:

Description of changes:

Merge Checklist

Put an x in 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.

  • I have read the CONTRIBUTING doc
  • I used the commit message format described in CONTRIBUTING
  • I have added tests that prove my fix is effective or that my feature works (if appropriate)
  • I have updated any necessary documentation (if appropriate)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository


echo "Mounting EFS File Systems"
sudo yum install -y amazon-efs-utils.noarch 0:1.10-1.amzn2
sudo yum install -y amazon-efs-utils

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

do different regions have different versions released?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

The list for the AMI ids are Amazon Linux AMI 2018, while the old hard-coded one is Amazon Linux2 AMI.
The new list needs amazon-efs-utils.noarch 0:1.10-1.amzn1 version. I just don't want specify specific version in case some of them need amazon-efs-utils.noarch 0:1.10-1.amzn2 version.
But it will automatically select the correct version if i don't specify

ROLE_NAME = "SageMakerRole"
REGION = "us-west-2"
EC2_INSTANCE_TYPE = "t2.micro"
AMI_ID = "ami-082b5a644766e0e6f"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think you can remove this

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Forgot to remove it

KEY_PATH = os.path.join(tempfile.gettempdir(), FILE_NAME)
STORAGE_CAPACITY_IN_BYTES = 3600

AWSRegionArch2AMI = {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

rename this to REGION_TO_AMI_MAP - we still want to honor Python capitalization/naming convention

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Sure

"cn-north-1": "ami-0a4eaf6c4454eda75",
"cn-northwest-1": "ami-6b6a7d09",
"us-gov-west-1": "ami-906cf0f1",
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@caxiaohu@laurenyu Is there any way we could make this dynamic based on the AMI name or type? Since we only need a basic AL AMI?
i.e.: Some dynamic logic that searches for an AMI by name and uses whatever ID is appropriate in that region. Eliminating the need for a map that we need to maintain.

I'm concerned that we'll need to add to this list for all future regions. This is likely another time-bomb that'll trigger a page on region expansion (or the dev will catch it during region expansion).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

good catch. I'd forgotten that I found this yesterday; I think it should do what we want: https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/finding-an-ami.html#finding-quick-start-ami

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Done!

@caxiaohucaxiaohuAug 27, 2019

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

The AMI ids search results:

RegionAMI id
us-west-1ami-0fcdcdb074d2bac5f
us-west-2ami-0f2176987ee50226e
us-east-1ami-035b3c7efe6d061d5
us-east-2ami-02f706d959cedf892
eu-west-1ami-0862aabda3fb488b5
eu-west-2ami-0bdfa1adc3878cd23
eu-west-3ami-05b93cd5a1b552734
eu-central-1ami-026d3b3672c6e7b66
ap-northeast-1ami-04b2d1589ab1d972c
ap-northeast-2ami-0be3e6f84d3b968cd
ap-northeast-3ami-0639e0e4c18187b30
ap-southeast-1ami-0fb6b6f9e81056553
ap-southeast-2ami-075caa3491def750b
ap-south-1ami-0b99c7725b9484f9e
ca-central-1ami-0a67d15f2858e33cb
sa-east-1ami-0bb96001cf2299257
cn-north-1AWS was not able to validate the provided access credentials
cn-northwest-1AWS was not able to validate the provided access credentials
us-gov-west-1AWS was not able to validate the provided access credentials

All of them are Amazon Linux AMI 2018.03.0

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

return fs_resources


def _dynamic_ami_id(sagemaker_session):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

let's rename this to _ami_id_for_region

def _dynamic_ami_id(sagemaker_session):
ec2_client = sagemaker_session.boto_session.client("ec2")
filters = [
{"Name": "name", "Values": ["amzn-ami-hvm-????.??.?.????????-x86_64-gp2"]},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

let's make this a constant. that'll both make it easier to find if it needs to be changed later for whatever reason and help describe what the string is.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Sure

image_details = sorted(response["Images"], key=itemgetter("CreationDate"), reverse=True)
if len(image_details) > 0:
ami_id = image_details[0]["ImageId"]
return ami_id

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: you can combine ll. 126-127

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Sure

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

STORAGE_CAPACITY_IN_BYTES = 3600

AMI_FILTERS = [
{"Name": "name", "Values": ["amzn-ami-hvm-????.??.?.????????-x86_64-gp2"]},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

sorry for the confusion. I meant specifically "amzn-ami-hvm-????.??.?.????????-x86_64-gp2" - it's not really obvious what this string means

@caxiaohucaxiaohuAug 27, 2019

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I see what you mean now

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@laurenyulaurenyu changed the title fix: changed AMI ids to be dynamic based on regionsfix: change AMI ids in tests to be dynamic based on regionsAug 27, 2019
@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@chuyang-deng
chuyang-deng merged commit f233dc0 into aws:masterAug 28, 2019
@chuyang-dengchuyang-deng mentioned this pull request Aug 28, 2019
4 tasks
shenlongtang added a commit to shenlongtang/sagemaker-python-sdk that referenced this pull request Oct 3, 2023
zuoyuanh added a commit that referenced this pull request Oct 5, 2023
* feature: method to build pipeline parameters from existing execution … (#951)
* feature: method to build pipeline parameters from existing execution with optional value overrides
* fix style check
* assert error message in unit test
* feature: allow opt out from referencing latest execution in the selec… (#1004)
* fix: Update pipeline.py and selective_execution_config.py with small fixes (#1099)
---------
Co-authored-by: stacicho <stacicho@amazon.com>
Co-authored-by: Zuoyuan Huang <david.hzy97@gmail.com>
Evan-W-ang added a commit to Evan-W-ang/sagemaker-python-sdk that referenced this pull request Jun 8, 2026
* fix: change Amazon AMI ids to be dynamic based on regions using searching Amazon Linux AMI
Evan-W-ang added a commit to Evan-W-ang/sagemaker-python-sdk that referenced this pull request Jun 8, 2026
* feature: method to build pipeline parameters from existing execution … (aws#951)
* feature: method to build pipeline parameters from existing execution with optional value overrides
* fix style check
* assert error message in unit test
* feature: allow opt out from referencing latest execution in the selec… (aws#1004)
* fix: Update pipeline.py and selective_execution_config.py with small fixes (aws#1099)
---------
Co-authored-by: stacicho <stacicho@amazon.com>
Co-authored-by: Zuoyuan Huang <david.hzy97@gmail.com>
Sign up for freeto 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.

5 participants

@caxiaohu@sagemaker-bot@laurenyu@chuyang-deng@knakad
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

fix: change AMI ids in tests to be dynamic based on regions - #1004

Merged
chuyang-deng merged 10 commits into
aws:masterfrom
caxiaohu:fix_region_issue
Aug 28, 2019
Merged

fix: change AMI ids in tests to be dynamic based on regions#1004
chuyang-deng merged 10 commits into
aws:masterfrom
caxiaohu:fix_region_issue

Conversation

@caxiaohu

@caxiaohucaxiaohu commented Aug 27, 2019

Copy link
Copy Markdown
Contributor

Issue #, if available:

Description of changes:

Merge Checklist

Put an x in 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.

  • I have read the CONTRIBUTING doc
  • I used the commit message format described in CONTRIBUTING
  • I have added tests that prove my fix is effective or that my feature works (if appropriate)
  • I have updated any necessary documentation (if appropriate)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository


echo "Mounting EFS File Systems"
sudo yum install -y amazon-efs-utils.noarch 0:1.10-1.amzn2
sudo yum install -y amazon-efs-utils

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

do different regions have different versions released?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

The list for the AMI ids are Amazon Linux AMI 2018, while the old hard-coded one is Amazon Linux2 AMI.
The new list needs amazon-efs-utils.noarch 0:1.10-1.amzn1 version. I just don't want specify specific version in case some of them need amazon-efs-utils.noarch 0:1.10-1.amzn2 version.
But it will automatically select the correct version if i don't specify

ROLE_NAME = "SageMakerRole"
REGION = "us-west-2"
EC2_INSTANCE_TYPE = "t2.micro"
AMI_ID = "ami-082b5a644766e0e6f"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think you can remove this

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Forgot to remove it

KEY_PATH = os.path.join(tempfile.gettempdir(), FILE_NAME)
STORAGE_CAPACITY_IN_BYTES = 3600

AWSRegionArch2AMI = {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

rename this to REGION_TO_AMI_MAP - we still want to honor Python capitalization/naming convention

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Sure

"cn-north-1": "ami-0a4eaf6c4454eda75",
"cn-northwest-1": "ami-6b6a7d09",
"us-gov-west-1": "ami-906cf0f1",
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@caxiaohu@laurenyu Is there any way we could make this dynamic based on the AMI name or type? Since we only need a basic AL AMI?
i.e.: Some dynamic logic that searches for an AMI by name and uses whatever ID is appropriate in that region. Eliminating the need for a map that we need to maintain.

I'm concerned that we'll need to add to this list for all future regions. This is likely another time-bomb that'll trigger a page on region expansion (or the dev will catch it during region expansion).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

good catch. I'd forgotten that I found this yesterday; I think it should do what we want: https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/finding-an-ami.html#finding-quick-start-ami

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Done!

@caxiaohucaxiaohuAug 27, 2019

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

The AMI ids search results:

RegionAMI id
us-west-1ami-0fcdcdb074d2bac5f
us-west-2ami-0f2176987ee50226e
us-east-1ami-035b3c7efe6d061d5
us-east-2ami-02f706d959cedf892
eu-west-1ami-0862aabda3fb488b5
eu-west-2ami-0bdfa1adc3878cd23
eu-west-3ami-05b93cd5a1b552734
eu-central-1ami-026d3b3672c6e7b66
ap-northeast-1ami-04b2d1589ab1d972c
ap-northeast-2ami-0be3e6f84d3b968cd
ap-northeast-3ami-0639e0e4c18187b30
ap-southeast-1ami-0fb6b6f9e81056553
ap-southeast-2ami-075caa3491def750b
ap-south-1ami-0b99c7725b9484f9e
ca-central-1ami-0a67d15f2858e33cb
sa-east-1ami-0bb96001cf2299257
cn-north-1AWS was not able to validate the provided access credentials
cn-northwest-1AWS was not able to validate the provided access credentials
us-gov-west-1AWS was not able to validate the provided access credentials

All of them are Amazon Linux AMI 2018.03.0

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

return fs_resources


def _dynamic_ami_id(sagemaker_session):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

let's rename this to _ami_id_for_region

def _dynamic_ami_id(sagemaker_session):
ec2_client = sagemaker_session.boto_session.client("ec2")
filters = [
{"Name": "name", "Values": ["amzn-ami-hvm-????.??.?.????????-x86_64-gp2"]},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

let's make this a constant. that'll both make it easier to find if it needs to be changed later for whatever reason and help describe what the string is.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Sure

image_details = sorted(response["Images"], key=itemgetter("CreationDate"), reverse=True)
if len(image_details) > 0:
ami_id = image_details[0]["ImageId"]
return ami_id

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: you can combine ll. 126-127

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Sure

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

STORAGE_CAPACITY_IN_BYTES = 3600

AMI_FILTERS = [
{"Name": "name", "Values": ["amzn-ami-hvm-????.??.?.????????-x86_64-gp2"]},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

sorry for the confusion. I meant specifically "amzn-ami-hvm-????.??.?.????????-x86_64-gp2" - it's not really obvious what this string means

@caxiaohucaxiaohuAug 27, 2019

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I see what you mean now

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@laurenyulaurenyu changed the title fix: changed AMI ids to be dynamic based on regionsfix: change AMI ids in tests to be dynamic based on regionsAug 27, 2019
@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@chuyang-deng
chuyang-deng merged commit f233dc0 into aws:masterAug 28, 2019
@chuyang-dengchuyang-deng mentioned this pull request Aug 28, 2019
4 tasks
shenlongtang added a commit to shenlongtang/sagemaker-python-sdk that referenced this pull request Oct 3, 2023
zuoyuanh added a commit that referenced this pull request Oct 5, 2023
* feature: method to build pipeline parameters from existing execution … (#951)
* feature: method to build pipeline parameters from existing execution with optional value overrides
* fix style check
* assert error message in unit test
* feature: allow opt out from referencing latest execution in the selec… (#1004)
* fix: Update pipeline.py and selective_execution_config.py with small fixes (#1099)
---------
Co-authored-by: stacicho <stacicho@amazon.com>
Co-authored-by: Zuoyuan Huang <david.hzy97@gmail.com>
Evan-W-ang added a commit to Evan-W-ang/sagemaker-python-sdk that referenced this pull request Jun 8, 2026
* fix: change Amazon AMI ids to be dynamic based on regions using searching Amazon Linux AMI
Evan-W-ang added a commit to Evan-W-ang/sagemaker-python-sdk that referenced this pull request Jun 8, 2026
* feature: method to build pipeline parameters from existing execution … (aws#951)
* feature: method to build pipeline parameters from existing execution with optional value overrides
* fix style check
* assert error message in unit test
* feature: allow opt out from referencing latest execution in the selec… (aws#1004)
* fix: Update pipeline.py and selective_execution_config.py with small fixes (aws#1099)
---------
Co-authored-by: stacicho <stacicho@amazon.com>
Co-authored-by: Zuoyuan Huang <david.hzy97@gmail.com>
Sign up for freeto 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.

5 participants

@caxiaohu@sagemaker-bot@laurenyu@chuyang-deng@knakad
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

fix: change AMI ids in tests to be dynamic based on regions - #1004

Merged
chuyang-deng merged 10 commits into
aws:masterfrom
caxiaohu:fix_region_issue
Aug 28, 2019
Merged

fix: change AMI ids in tests to be dynamic based on regions#1004
chuyang-deng merged 10 commits into
aws:masterfrom
caxiaohu:fix_region_issue

Conversation

@caxiaohu

@caxiaohucaxiaohu commented Aug 27, 2019

Copy link
Copy Markdown
Contributor

Issue #, if available:

Description of changes:

Merge Checklist

Put an x in 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.

  • I have read the CONTRIBUTING doc
  • I used the commit message format described in CONTRIBUTING
  • I have added tests that prove my fix is effective or that my feature works (if appropriate)
  • I have updated any necessary documentation (if appropriate)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository


echo "Mounting EFS File Systems"
sudo yum install -y amazon-efs-utils.noarch 0:1.10-1.amzn2
sudo yum install -y amazon-efs-utils

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

do different regions have different versions released?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

The list for the AMI ids are Amazon Linux AMI 2018, while the old hard-coded one is Amazon Linux2 AMI.
The new list needs amazon-efs-utils.noarch 0:1.10-1.amzn1 version. I just don't want specify specific version in case some of them need amazon-efs-utils.noarch 0:1.10-1.amzn2 version.
But it will automatically select the correct version if i don't specify

ROLE_NAME = "SageMakerRole"
REGION = "us-west-2"
EC2_INSTANCE_TYPE = "t2.micro"
AMI_ID = "ami-082b5a644766e0e6f"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think you can remove this

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Forgot to remove it

KEY_PATH = os.path.join(tempfile.gettempdir(), FILE_NAME)
STORAGE_CAPACITY_IN_BYTES = 3600

AWSRegionArch2AMI = {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

rename this to REGION_TO_AMI_MAP - we still want to honor Python capitalization/naming convention

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Sure

"cn-north-1": "ami-0a4eaf6c4454eda75",
"cn-northwest-1": "ami-6b6a7d09",
"us-gov-west-1": "ami-906cf0f1",
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@caxiaohu@laurenyu Is there any way we could make this dynamic based on the AMI name or type? Since we only need a basic AL AMI?
i.e.: Some dynamic logic that searches for an AMI by name and uses whatever ID is appropriate in that region. Eliminating the need for a map that we need to maintain.

I'm concerned that we'll need to add to this list for all future regions. This is likely another time-bomb that'll trigger a page on region expansion (or the dev will catch it during region expansion).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

good catch. I'd forgotten that I found this yesterday; I think it should do what we want: https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/finding-an-ami.html#finding-quick-start-ami

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Done!

@caxiaohucaxiaohuAug 27, 2019

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

The AMI ids search results:

RegionAMI id
us-west-1ami-0fcdcdb074d2bac5f
us-west-2ami-0f2176987ee50226e
us-east-1ami-035b3c7efe6d061d5
us-east-2ami-02f706d959cedf892
eu-west-1ami-0862aabda3fb488b5
eu-west-2ami-0bdfa1adc3878cd23
eu-west-3ami-05b93cd5a1b552734
eu-central-1ami-026d3b3672c6e7b66
ap-northeast-1ami-04b2d1589ab1d972c
ap-northeast-2ami-0be3e6f84d3b968cd
ap-northeast-3ami-0639e0e4c18187b30
ap-southeast-1ami-0fb6b6f9e81056553
ap-southeast-2ami-075caa3491def750b
ap-south-1ami-0b99c7725b9484f9e
ca-central-1ami-0a67d15f2858e33cb
sa-east-1ami-0bb96001cf2299257
cn-north-1AWS was not able to validate the provided access credentials
cn-northwest-1AWS was not able to validate the provided access credentials
us-gov-west-1AWS was not able to validate the provided access credentials

All of them are Amazon Linux AMI 2018.03.0

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

return fs_resources


def _dynamic_ami_id(sagemaker_session):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

let's rename this to _ami_id_for_region

def _dynamic_ami_id(sagemaker_session):
ec2_client = sagemaker_session.boto_session.client("ec2")
filters = [
{"Name": "name", "Values": ["amzn-ami-hvm-????.??.?.????????-x86_64-gp2"]},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

let's make this a constant. that'll both make it easier to find if it needs to be changed later for whatever reason and help describe what the string is.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Sure

image_details = sorted(response["Images"], key=itemgetter("CreationDate"), reverse=True)
if len(image_details) > 0:
ami_id = image_details[0]["ImageId"]
return ami_id

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: you can combine ll. 126-127

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Sure

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

STORAGE_CAPACITY_IN_BYTES = 3600

AMI_FILTERS = [
{"Name": "name", "Values": ["amzn-ami-hvm-????.??.?.????????-x86_64-gp2"]},

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

sorry for the confusion. I meant specifically "amzn-ami-hvm-????.??.?.????????-x86_64-gp2" - it's not really obvious what this string means

@caxiaohucaxiaohuAug 27, 2019

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I see what you mean now

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@laurenyulaurenyu changed the title fix: changed AMI ids to be dynamic based on regionsfix: change AMI ids in tests to be dynamic based on regionsAug 27, 2019
@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@sagemaker-bot

Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@chuyang-deng
chuyang-deng merged commit f233dc0 into aws:masterAug 28, 2019
@chuyang-dengchuyang-deng mentioned this pull request Aug 28, 2019
4 tasks
shenlongtang added a commit to shenlongtang/sagemaker-python-sdk that referenced this pull request Oct 3, 2023
zuoyuanh added a commit that referenced this pull request Oct 5, 2023
* feature: method to build pipeline parameters from existing execution … (#951)
* feature: method to build pipeline parameters from existing execution with optional value overrides
* fix style check
* assert error message in unit test
* feature: allow opt out from referencing latest execution in the selec… (#1004)
* fix: Update pipeline.py and selective_execution_config.py with small fixes (#1099)
---------
Co-authored-by: stacicho <stacicho@amazon.com>
Co-authored-by: Zuoyuan Huang <david.hzy97@gmail.com>
Evan-W-ang added a commit to Evan-W-ang/sagemaker-python-sdk that referenced this pull request Jun 8, 2026
* fix: change Amazon AMI ids to be dynamic based on regions using searching Amazon Linux AMI
Evan-W-ang added a commit to Evan-W-ang/sagemaker-python-sdk that referenced this pull request Jun 8, 2026
* feature: method to build pipeline parameters from existing execution … (aws#951)
* feature: method to build pipeline parameters from existing execution with optional value overrides
* fix style check
* assert error message in unit test
* feature: allow opt out from referencing latest execution in the selec… (aws#1004)
* fix: Update pipeline.py and selective_execution_config.py with small fixes (aws#1099)
---------
Co-authored-by: stacicho <stacicho@amazon.com>
Co-authored-by: Zuoyuan Huang <david.hzy97@gmail.com>
Sign up for freeto 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.

5 participants

@caxiaohu@sagemaker-bot@laurenyu@chuyang-deng@knakad