Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

@aws-cdk/aws-ecs-patterns: minScalingCapacity cannot be set to 0 #15632

Closed
paya-cz opened this issue Jul 18, 2021 · 5 comments · Fixed by #16961
Closed

@aws-cdk/aws-ecs-patterns: minScalingCapacity cannot be set to 0 #15632

paya-cz opened this issue Jul 18, 2021 · 5 comments · Fixed by #16961
Assignees
Labels
@aws-cdk/aws-ecs-patterns Related to ecs-patterns library bug This issue is a bug. effort/small Small work item – less than a day of effort p2

Comments

@paya-cz
Copy link

paya-cz commented Jul 18, 2021

Reproduction Steps

It is impossible to set minScalingCapacity on a SQS-processing Fargate task to 0, because of this check:

this.minCapacity = props.minScalingCapacity || this.desiredCount;

The operator || treats 0 as a falsy value. Use ?? instead. A workaround seems to be to set desiredTaskCount deprecated prop to 0, since that's used in the falsy branch path.

What did you expect to happen?

minScalingCapacity set to 0 should deploy autoscaling with minimum tasks: 0.

What actually happened?

minScalingCapacity set to 0 deploys ECS autoscaling with minimum tasks: 1.

Environment

  • CDK CLI Version : 1.114.0 (build 7e41b6b)
  • Framework Version: 1.114.0
  • Node.js Version: v14.17.1
  • OS : Win 10
  • Language (Version): TypeScript (3.9.10)

This is 🐛 Bug Report

@paya-cz paya-cz added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jul 18, 2021
@github-actions github-actions bot added the @aws-cdk/aws-ecs-patterns Related to ecs-patterns library label Jul 18, 2021
@peterwoodworth peterwoodworth removed the needs-triage This issue or PR still needs to be triaged. label Sep 20, 2021
@peterwoodworth
Copy link
Contributor

Hey @paya-cz, would you like to submit a pr for this?

@peterwoodworth peterwoodworth added p2 effort/small Small work item – less than a day of effort labels Sep 20, 2021
@paya-cz
Copy link
Author

paya-cz commented Sep 21, 2021

@peterwoodworth I tried to, but it looks like getting the dev environment up and running is quite a bit more involved than I hoped for. I opened the repo in Gitpod, but the build returned error code 1 with the error message being buried somewhere deep in the log and not available in the terminal anymore.

Trying to run scripts/foreach.sh yarn build gives me error error Command "build" not found.

As a last resort, I did cd into /packages/@aws-cdk/aws-ecs-patterns and tried to yarn build, only to get a failure message with @aws-cdk/aws-ecs not being built. Attempting to build that, got another error with @aws-cdk/aws-s3-deployment. Not sure how deep that chain goes, seems like I would have to build a ton of packages by hand one by one.

Seems to me the contributing guide is either outdated, or I must have missed some step? Although I imagine the point of Gitpod is that everything should work out of the box. So I don't think I will be able to submit a PR for this.

@peterwoodworth
Copy link
Contributor

I've never used the Gitpod method, so I don't think I'll be able to troubleshoot that for you. Have you tried the other method of submitting PRs? I could help you with that if you'd like. If not, I can try to get a PR up for this when I have the time

@paya-cz
Copy link
Author

paya-cz commented Oct 5, 2021

@peterwoodworth I see you already made a PR for this, thank you for handling this issue.

I use Windows VM for development, I guess I might be able to get WSL setup in order to contribute to the project, but unfortunately don't really have time to play with that at the moment. Gitpod would solve this friction beautifully though if it worked :-)

@mergify mergify bot closed this as completed in #16961 Oct 13, 2021
mergify bot pushed a commit that referenced this issue Oct 13, 2021
fixes: #15632
fixes: #14336

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

TikiTDO pushed a commit to TikiTDO/aws-cdk that referenced this issue Feb 21, 2022
fixes: aws#15632
fixes: aws#14336

----

*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
Labels
@aws-cdk/aws-ecs-patterns Related to ecs-patterns library bug This issue is a bug. effort/small Small work item – less than a day of effort p2
Projects
None yet
5 participants