Skip to content

fix(ecs): enabling the circuitBreaker is not recommended loudly enough - #37755

Merged
mergify[bot] merged 7 commits into
mainfrom
huijbers/recommend-circuit-breaker
May 6, 2026
Merged

fix(ecs): enabling the circuitBreaker is not recommended loudly enough#37755
mergify[bot] merged 7 commits into
mainfrom
huijbers/recommend-circuit-breaker

Conversation

@rix0rrr

@rix0rrr rix0rrr commented May 4, 2026

Copy link
Copy Markdown
Contributor

If a simple misconfiguration in an ECS Task Definition causes the task to never start, a Service based off of that Task Definition takes 3 hours to fail unless circuitBreaker is configured.

This is not obvious: our examples don't show that and the construct itself never tells you about it.

Update our top-level README example to include the circuit breaker field, and make the construct warn you if you didn't configure the property (you most likely always want it). Also remove desiredCapacity from the initial example, which is not recommended anymore because setting that unscales your service during deployments (it even warns).


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

If a simple misconfiguration in an ECS Task Definition causes the task
to never start, a Service based off of that Task Definition takes 3
hours to fail unless `circuitBreaker` is configured.

This is not obvious: our examples don't show that and the construct
itself never tells you about it.

Update our top-level README example to include the circuit breaker
field, and make the construct warn you if you didn't configure the
property (you most likely always want it).
@rix0rrr
rix0rrr requested a review from a team May 4, 2026 12:12
@aws-cdk-automation
aws-cdk-automation requested a review from a team May 4, 2026 12:12
@github-actions github-actions Bot added the p2 label May 4, 2026
@rix0rrr rix0rrr changed the title fix(ecs): suggest setting circuitBreaker if not configured fix(ecs): enabling the circuitBreaker is not recommended loudly enough May 4, 2026
@mergify mergify Bot added the contribution/core This is a PR that came from AWS. label May 4, 2026
@mergify
mergify Bot temporarily deployed to automation May 4, 2026 12:13 Inactive
@mergify
mergify Bot temporarily deployed to automation May 4, 2026 12:13 Inactive
@github-actions

github-actions Bot commented May 4, 2026

Copy link
Copy Markdown
Contributor

⚠️ This pull request description does not follow the correct template structure.

PRs without a linked issue will receive lower priority for review and merging. Please update the description to follow the PR template and include a line like Closes #123 in the Issue section. If no existing issue matches your change, create one first.

@aws-cdk-automation aws-cdk-automation left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

(This review is outdated)

@rix0rrr rix0rrr added the pr-linter/exempt-integ-test The PR linter will not require integ test changes label May 4, 2026
@aws-cdk-automation
aws-cdk-automation dismissed their stale review May 4, 2026 14:11

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

@mergify

mergify Bot commented May 6, 2026

Copy link
Copy Markdown
Contributor

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

@mergify

mergify Bot commented May 6, 2026

Copy link
Copy Markdown
Contributor

Merge Queue Status

  • Entered queue2026-05-06 09:14 UTC · Rule: default-squash
  • Checks passed · in-place
  • Merged2026-05-06 10:54 UTC · at 18de8e741dbcdd4cabea91849709debe4289da6f · squash

This pull request spent 1 hour 39 minutes 37 seconds in the queue, including 31 minutes 56 seconds running CI.

Required conditions to merge

@mergify
mergify Bot temporarily deployed to automation May 6, 2026 09:49 Inactive
@mergify
mergify Bot temporarily deployed to automation May 6, 2026 09:49 Inactive
@mergify

mergify Bot commented May 6, 2026

Copy link
Copy Markdown
Contributor

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

@mergify
mergify Bot temporarily deployed to automation May 6, 2026 10:22 Inactive
@mergify
mergify Bot temporarily deployed to automation May 6, 2026 10:22 Inactive
@mergify

mergify Bot commented May 6, 2026

Copy link
Copy Markdown
Contributor

Thank you for contributing! Your pull request will be updated from main 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 a52af7d into main May 6, 2026
18 of 19 checks passed
@mergify
mergify Bot deleted the huijbers/recommend-circuit-breaker branch May 6, 2026 10:54
@github-actions

github-actions Bot commented May 6, 2026

Copy link
Copy Markdown
Contributor

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

@github-actions github-actions Bot locked as resolved and limited conversation to collaborators May 6, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

contribution/core This is a PR that came from AWS. p2 pr-linter/exempt-integ-test The PR linter will not require integ test changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants