Skip to content

fix(ecs): cannot use tokens for certain props#17106

Closed
peterwoodworth wants to merge 4 commits intoaws:masterfrom
peterwoodworth:token-patch-1
Closed

fix(ecs): cannot use tokens for certain props#17106
peterwoodworth wants to merge 4 commits intoaws:masterfrom
peterwoodworth:token-patch-1

Conversation

@peterwoodworth
Copy link
Copy Markdown
Contributor

fixes #16648


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
Copy Markdown

gitpod-io bot commented Oct 22, 2021

@github-actions github-actions bot added the @aws-cdk/aws-ecs Related to Amazon Elastic Container label Oct 22, 2021
@peterwoodworth peterwoodworth added the contribution/core This is a PR that came from AWS. label Oct 26, 2021
@aws-cdk-automation
Copy link
Copy Markdown
Collaborator

AWS CodeBuild CI Report

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

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

Copy link
Copy Markdown
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.

Looks great, I think we should just add one more test. Thanks, @peterwoodworth!

this.cluster = props.cluster || this.getDefaultCluster(this, props.vpc);

if (props.desiredCount !== undefined && props.desiredCount < 1) {
if (props.desiredCount !== undefined && !cdk.Token.isUnresolved(props.desiredCount) && props.desiredCount < 1) {
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.

Can you add a test for this one as well?

@rix0rrr rix0rrr added bug This issue is a bug. p1 and removed @aws-cdk/aws-ecs Related to Amazon Elastic Container contribution/core This is a PR that came from AWS. labels Mar 4, 2022
@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Mar 4, 2022
@peterwoodworth peterwoodworth deleted the token-patch-1 branch March 17, 2022 23:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug This issue is a bug. contribution/core This is a PR that came from AWS. p1

Projects

None yet

Development

Successfully merging this pull request may close these issues.

(ecs.FargateTaskDefinition): runtime check for EphemeralStorageGiB makes it incompatible with CfnParameter tokens

5 participants