Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat(aws-ecs): support runtime platform property for create fargate windows runtime. #17622

Merged
merged 26 commits into from
Jan 11, 2022
Merged

feat(aws-ecs): support runtime platform property for create fargate windows runtime. #17622

merged 26 commits into from
Jan 11, 2022

Conversation

neilkuan
Copy link
Contributor

@neilkuan neilkuan commented Nov 22, 2021 •
edited
Loading

feat(aws-ecs): support runtime platform property for create fargate windows and Graviton2 runtime.

close #17242

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

@gitpod-io
Copy link

gitpod-io bot commented Nov 22, 2021

@github-actions github-actions bot added the @aws-cdk/aws-ecs Related to Amazon Elastic Container label Nov 22, 2021
@neilkuan
Copy link
Contributor Author

Hi @upparekh ,If you have time, please take a look at this PR, thanks. ☺️

@pahud
Copy link
Contributor

pahud commented Nov 24, 2021 •
edited
Loading

Hi @kimisme9386 @clarencetw @dwchiang

Are you interested to take an initial review? Please leave a review comment with "LGTM" if you think it's good with you.

Thanks.

@pahud
Copy link
Contributor

pahud commented Nov 24, 2021

@neilkuan Any chance we can add this new feature into this PR?

Announcing AWS Graviton2 Support for AWS Fargate – Get up to 40% Better Price-Performance for Your Serverless Containers
https://aws.amazon.com/tw/blogs/aws/announcing-aws-graviton2-support-for-aws-fargate-get-up-to-40-better-price-performance-for-your-serverless-containers/

@neilkuan
Copy link
Contributor Author

@neilkuan Any chance we can add this new feature into this PR?

Announcing AWS Graviton2 Support for AWS Fargate – Get up to 40% Better Price-Performance for Your Serverless Containers https://aws.amazon.com/tw/blogs/aws/announcing-aws-graviton2-support-for-aws-fargate-get-up-to-40-better-price-performance-for-your-serverless-containers/

yes, I will, wip.

kimisme9386
kimisme9386 reviewed Nov 24, 2021
View reviewed changes
Copy link
Contributor

@kimisme9386 kimisme9386 left a comment

Choose a reason for hiding this comment

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

LGTM

clarencetw
clarencetw reviewed Nov 24, 2021
View reviewed changes
Copy link
Contributor

@clarencetw clarencetw left a comment

Choose a reason for hiding this comment

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

LGTM

dwchiang
dwchiang reviewed Nov 24, 2021
View reviewed changes
Copy link

@dwchiang dwchiang left a comment

Choose a reason for hiding this comment

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

LGTM. It's great to have Graviton2 for AWS Fargate.

madeline-k
madeline-k previously requested changes Nov 24, 2021
View reviewed changes
Copy link
Contributor

@madeline-k madeline-k left a comment

Choose a reason for hiding this comment

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

Thanks for opening this PR, @neilkuan! I have a few questions/comments.

@mergify mergify bot dismissed madeline-k’s stale review November 24, 2021 09:30

Pull request has been modified.

@neilkuan neilkuan requested a review from madeline-k November 24, 2021 15:14
neilkuan and others added 2 commits November 25, 2021 12:13
@neilkuan @pahud
Update packages/@aws-cdk/aws-ecs/lib/base/task-definition.ts
33cf68c 
Co-authored-by: Pahud Hsieh <pahudnet@gmail.com>
@neilkuan @pahud
Update packages/@aws-cdk/aws-ecs/lib/base/task-definition.ts
d4e704b 
Co-authored-by: Pahud Hsieh <pahudnet@gmail.com>
@mergify mergify bot dismissed pahud’s stale review November 25, 2021 04:14

Pull request has been modified.

@neilkuan
Copy link
Contributor Author

Hi @madeline-k , I already made some improvement. please review at the free time, thanks.

@neilkuan
Copy link
Contributor Author

Hi @madeline-k , I already made some improvement. please review at the free time, thanks.

madeline-k
madeline-k previously requested changes Jan 5, 2022
View reviewed changes
Copy link
Contributor

@madeline-k madeline-k left a comment

Choose a reason for hiding this comment

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

Thanks for making updates, @neilkuan! This is looking great, I just have a few comments about wording and unit tests.

@mergify mergify bot dismissed madeline-k’s stale review January 6, 2022 02:15

Pull request has been modified.

@neilkuan neilkuan requested a review from madeline-k January 10, 2022 01:28
@neilkuan
Copy link
Contributor Author

Hi @madeline-k ready to merge. 😁

madeline-k
madeline-k previously approved these changes Jan 10, 2022
View reviewed changes
Copy link
Contributor

@madeline-k madeline-k left a comment

Choose a reason for hiding this comment

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

😄

@mergify mergify bot dismissed madeline-k’s stale review January 10, 2022 18:29

Pull request has been modified.

madeline-k
madeline-k approved these changes Jan 11, 2022
View reviewed changes
Copy link
Contributor

@madeline-k madeline-k left a comment

Choose a reason for hiding this comment

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

Approving, my review was dismissed after merging from master.

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
  • Commit ID: aef30c8
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@mergify
Copy link
Contributor

mergify bot commented Jan 11, 2022

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@mergify mergify bot merged commit fa8f2e2 into aws:master Jan 11, 2022
This was referenced Jan 13, 2022
Closed
Closed
TikiTDO pushed a commit to TikiTDO/aws-cdk that referenced this pull request Feb 21, 2022
@neilkuan @TikiTDO
feat(aws-ecs): support runtime platform property for create fargate w…
52b99bc 
…indows runtime. (aws#17622)

feat(aws-ecs): support runtime platform property for create fargate windows and Graviton2 runtime.

close aws#17242
----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dwchiang dwchiang dwchiang left review comments

@clarencetw clarencetw clarencetw left review comments

@kimisme9386 kimisme9386 kimisme9386 left review comments

@madeline-k madeline-k madeline-k approved these changes

@pahud pahud pahud left review comments

Assignees

@madeline-k madeline-k

Labels
@aws-cdk/aws-ecs Related to Amazon Elastic Container
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

(aws-ecs): support RuntimePlatform Property at TaskDefinition class for create windows OperatingSystem
7 participants
@neilkuan @pahud @aws-cdk-automation @dwchiang @clarencetw @kimisme9386 @madeline-k