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

aws-cdk-lib.aws_ecs_patterns module: REMOVE_DEFAULT_DESIRED_COUNT is no longer supported in cdkv2 #29325

Closed
jlizen opened this issue Mar 1, 2024 · 3 comments
Closed

aws-cdk-lib.aws_ecs_patterns module: REMOVE_DEFAULT_DESIRED_COUNT is no longer supported in cdkv2 #29325

jlizen opened this issue Mar 1, 2024 · 3 comments
Labels
aws-cdk-lib Related to the aws-cdk-lib package documentation This is a problem with documentation. effort/small Small work item – less than a day of effort p2

Comments

@jlizen
Copy link
Contributor

jlizen commented Mar 1, 2024 •
edited
Loading

Describe the issue

Currently the aws_ecs_patterns module docs reference the REMOVE_DEFAULT_DESIRED_COUNT flag in a few places:

The overall readme, on the feature flag:
https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_ecs_patterns-readme.html#use-the-remove_default_desired_count-feature-flag

Similarly, individual patterns' docs reference the flag when discussing the desiredCount property, eg:
https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_ecs_patterns.ApplicationLoadBalancedFargateService.html#desiredcount

Type: number (optional, default: If the feature flag, ECS_REMOVE_DEFAULT_DESIRED_COUNT is false, the default is 1; if true, the default is 1 for all new services and uses the existing services desired count when updating an existing service.)

However, when building a package using it, I see this error:

 Error: Unsupported feature flag '@aws-cdk/aws-ecs-patterns:removeDefaultDesiredCount'. This flag existed on CDKv1 but has been removed in CDKv2. CDK will now behave as the same as when the flag is enabled.

You can see it on the unsupported list for v2 here.

These references in the docs should be removed, in favor of describing the (now unconditional) behavior with the feature enabled.

Links

https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_ecs_patterns-readme.html#use-the-remove_default_desired_count-feature-flag

https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_ecs_patterns.ApplicationLoadBalancedFargateService.html#desiredcount

https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_ecs_patterns.ApplicationLoadBalancedEc2Service.html#desiredcount

https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_ecs_patterns.ApplicationMultipleTargetGroupsEc2Service.html#desiredcount

https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_ecs_patterns.ApplicationMultipleTargetGroupsFargateService.html#desiredcount

https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_ecs_patterns.NetworkLoadBalancedEc2Service.html#desiredcount

https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_ecs_patterns.NetworkLoadBalancedFargateService.html#desiredcount

https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_ecs_patterns.NetworkMultipleTargetGroupsEc2Service.html#desiredcount

https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_ecs_patterns.NetworkMultipleTargetGroupsFargateService.html#desiredcount
@jlizen jlizen added documentation This is a problem with documentation. needs-triage This issue or PR still needs to be triaged. labels Mar 1, 2024
@github-actions github-actions bot added the aws-cdk-lib Related to the aws-cdk-lib package label Mar 1, 2024
@pahud
Copy link
Contributor

pahud commented Mar 1, 2024

Thank you for the heads-up and report. I guess we should remove that from the doc. Are you interested to submit a PR for that?

@pahud pahud added p2 effort/small Small work item – less than a day of effort and removed effort/small Small work item – less than a day of effort needs-triage This issue or PR still needs to be triaged. labels Mar 1, 2024
@jlizen
Copy link
Contributor Author

jlizen commented Mar 2, 2024

Sure, coming up shortly.

@jlizen jlizen mentioned this issue Mar 2, 2024
Merged
1 task
mergify bot pushed a commit that referenced this issue Mar 4, 2024
@jlizen
docs(ecs-patterns): remove references to REMOVE_DEFAULT_DESIRED_COUNT (…
704f596 
…#29344)

### Issue # (if applicable)

Closes # 29325

### Reason for this change

The `REMOVE_DEFAULT_DESIRED_COUNT` feature flag is always enabled in CDKv2, and throws builds errors if explicitly set. The `ecs-patterns`  docs still reference it as "opt-in", which is misleading.

 Ref: [list of deprecated feature flags for v2](https://github.com/aws/aws-cdk/blob/3cbad4a2164a41f5529e04aba4d15085c71b7849/packages/aws-cdk-lib/cx-api/FEATURE_FLAGS.md?plain=1#L145)

See [Issue 29325](#29325) for a sample build error when trying to follow the current example code in docs for enabling the flag.

I did NOT remove the actual conditionals in the construct code, that check the (now always true) feature flag. This is dead code that can probably be removed as a chore task. My focus here was on removing friction for developers reading documentation.

### Description of changes

I removed the section in the README of `ecs-patterns` showing how to manually enable this flag. I also updated the default cases in docstrings that referenced the flag.

### Description of how you validated changes

Doc change only, no functional changes. I did double check that the defaults described in the docstrings (when the feature flag is enabled) were still accurate.

### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@jlizen jlizen closed this as completed Mar 4, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Mar 4, 2024

⚠️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.

@github-actions github-actions bot mentioned this issue Apr 1, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
aws-cdk-lib Related to the aws-cdk-lib package documentation This is a problem with documentation. effort/small Small work item – less than a day of effort p2
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@pahud @jlizen