Skip to content

Conversation

@jungseoklee
Copy link
Contributor

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

Summary

This patch fixes an AutoScalingGroup example in the README file.

Test

  • Deploy the sample snippet through cdk 0.12.0 (build 2dc324e).

Notes

  • It would be the best to update examples of README automatically if possible, but I do not have any good ideas.

This patch fixes an AutoScalingGroup example in the README file.

new ec2.AutoScalingGroup(stack, 'ASG', {
const vpc = new ec2.VpcNetwork(stack, 'VPC', {
maxAZs: 3
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to limit AZs in this example, just leave new ec2.VpcNetwork(stack, 'VPC')

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I will remove it.

machineImage: new ec2.LinuxImage({
'us-east-1': 'ami-97785bed'
})
machineImage: new ec2.AmazonLinuxImage()
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe a small comment explaining what this does

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I will add it. The AMI does not work for us-west-2. Specifically, I got the following message.

[AWS::AutoScaling::LaunchConfiguration] asgLaunchConfig37FDE42B AMI ami-97785bed is invalid: The image id '[ami-97785bed]' does not exist (Service: AmazonAutoScaling; Status Code: 400; Error Code: ValidationError; Request ID: d9ec164e-d35c-11e8-b28a-e7ee5dc94f2f)

Hence, I think that it is more robust to ec2.AmazonLinuxImage() as an example.

@rix0rrr
Copy link
Contributor

rix0rrr commented Oct 19, 2018

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants