fix: stop passing the deprecated property to Cfn construct by rantoniuk · Pull Request #35080 · aws/aws-cdk

rantoniuk · 2025-07-27T19:18:37Z

Follow-up of #34618 as it did not fully fix the warnings.

Change

Stop passing the deprecated property to Cfn object.

Tests

Tested on minimal stack and an actual production stack.

Fixes #34895

Copilot

Pull Request Overview

This PR removes lazy loading for the deprecated inferenceAccelerators property in ECS task definitions to eliminate warnings. The change simplifies the code by directly evaluating the inference accelerators condition instead of wrapping it in a lazy loader.

Replaces lazy-loaded inferenceAccelerators with direct evaluation
Maintains the same conditional logic for Fargate compatibility
Addresses deprecation warnings from the lazy loading mechanism

aws-cdk-automation

The pull request linter fails with the following errors:

❌ Fixes must contain a change to an integration test file and the resulting snapshot.

If you believe this pull request should receive an exemption, please comment and provide a justification. A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed, add Clarification Request to a comment.

✅ A exemption request has been requested. Please wait for a maintainer's review.

rantoniuk · 2025-07-27T19:24:47Z

Exemption Request - as previously, I don't think this needs tests.

aws-cdk-automation · 2025-07-27T19:38:06Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
Commit ID: 594ce5e
Result: FAILED
Build Logs (available for 30 days)

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

rantoniuk · 2025-08-18T12:15:44Z

@pahud any chance you can help finding a reviewer?

Abogical

I believe a unit test (at packages/aws-cdk-lib/aws-ecs/test/task-definition.test.ts) is needed here just to check that the inferenceAccelerator is not added by default and that it is only added iff an inferenceAccelerator is passed.

packages/aws-cdk-lib/aws-ecs/lib/base/task-definition.ts

rantoniuk · 2025-08-21T09:14:27Z

I believe a unit test (at packages/aws-cdk-lib/aws-ecs/test/task-definition.test.ts) is needed here just to check that the inferenceAccelerator is not added by default and that it is only added iff an inferenceAccelerator is passed.

This is already covered by unit tests.

Abogical · 2025-08-21T09:55:45Z

I believe a unit test (at packages/aws-cdk-lib/aws-ecs/test/task-definition.test.ts) is needed here just to check that the inferenceAccelerator is not added by default and that it is only added iff an inferenceAccelerator is passed.

This is already covered by unit tests.

Is there a unit test to test that inferenceAccelerator is not passed if not provided? (by using Match.absent for example)

Abogical

Per my comment above. A unit test to test that inferenceAccelerator is not passed is needed here. You can use Match.absent to achieve this.

Abogical

Integ test snapshots would need to be updated:

@aws-cdk-testing/framework-integ:   CHANGED    aws-dynamodb/test/integ.global-replicas-provisioned 2.532s
@aws-cdk-testing/framework-integ:       Resources
@aws-cdk-testing/framework-integ:       [~] AWS::CloudFormation::Stack awscdkawsdynamodbReplicaProviderNestedStackawscdkawsdynamodbReplicaProviderNestedStackResource18E3F12D
@aws-cdk-testing/framework-integ:        └─ [~] TemplateURL
@aws-cdk-testing/framework-integ:            └─ [~] .Fn::Join:
@aws-cdk-testing/framework-integ:                └─ @@ -13,6 +13,6 @@
@aws-cdk-testing/framework-integ:                   [ ]     {
@aws-cdk-testing/framework-integ:                   [ ]       "Fn::Sub": "cdk-hnb659fds-assets-${AWS::AccountId}-${AWS::Region}"
@aws-cdk-testing/framework-integ:                   [ ]     },
@aws-cdk-testing/framework-integ:                   [-]     "/be82eb3f30fbf19ad6227c31590fdcc98fdc3f1968a085e41f52ff22aaa2fff6.json"
@aws-cdk-testing/framework-integ:                   [+]     "/d80a42298fecb9cef8ab0548bbddde09f369cc7d09c1c6fabbae147532f77556.json"
@aws-cdk-testing/framework-integ:                   [ ]   ]
@aws-cdk-testing/framework-integ:                   [ ] ]
@aws-cdk-testing/framework-integ:       
@aws-cdk-testing/framework-integ:       
@aws-cdk-testing/framework-integ:   CHANGED    aws-dynamodb/test/integ.global-replicas-provisioned 2.532s
@aws-cdk-testing/framework-integ:       Resources
@aws-cdk-testing/framework-integ:       [~] AWS::Lambda::Function OnEventHandler42BEBAE0
@aws-cdk-testing/framework-integ:        └─ [~] Code
@aws-cdk-testing/framework-integ:            └─ [~] .S3Key:
@aws-cdk-testing/framework-integ:                ├─ [-] 7e256f5e4a92713cc232cd23819cb666756ee6023ce380130abac3b6a3fcea6a.zip
@aws-cdk-testing/framework-integ:                └─ [+] 7ecc77636deefb31954ea2e6aadf6d2c361a2943535a678d65f46a8de5e1f503.zip
@aws-cdk-testing/framework-integ:       [~] AWS::Lambda::Function IsCompleteHandler7073F4DA
@aws-cdk-testing/framework-integ:        └─ [~] Code
@aws-cdk-testing/framework-integ:            └─ [~] .S3Key:
@aws-cdk-testing/framework-integ:                ├─ [-] 7e256f5e4a92713cc232cd23819cb666756ee6023ce380130abac3b6a3fcea6a.zip
@aws-cdk-testing/framework-integ:                └─ [+] 7ecc77636deefb31954ea2e6aadf6d2c361a2943535a678d65f46a8de5e1f503.zip
@aws-cdk-testing/framework-integ:       
@aws-cdk-testing/framework-integ:       
@aws-cdk-testing/framework-integ:   CHANGED    aws-dynamodb/test/integ.global 2.561s
@aws-cdk-testing/framework-integ:       Resources
@aws-cdk-testing/framework-integ:       [~] AWS::CloudFormation::Stack awscdkawsdynamodbReplicaProviderNestedStackawscdkawsdynamodbReplicaProviderNestedStackResource18E3F12D
@aws-cdk-testing/framework-integ:        └─ [~] TemplateURL
@aws-cdk-testing/framework-integ:            └─ [~] .Fn::Join:
@aws-cdk-testing/framework-integ:                └─ @@ -9,6 +9,6 @@
@aws-cdk-testing/framework-integ:                   [ ]     {
@aws-cdk-testing/framework-integ:                   [ ]       "Fn::Sub": "cdk-hnb659fds-assets-${AWS::AccountId}-eu-west-1"
@aws-cdk-testing/framework-integ:                   [ ]     },
@aws-cdk-testing/framework-integ:                   [-]     "/2d2187472236ea907da93f0398d9237e06c266540258f4ed7ac24cd4ef9dcabb.json"
@aws-cdk-testing/framework-integ:                   [+]     "/85cd6b466d0c362b83cedb76074823dd89cccb47cbf551109abb90e8406bcd3d.json"
@aws-cdk-testing/framework-integ:                   [ ]   ]
@aws-cdk-testing/framework-integ:                   [ ] ]
@aws-cdk-testing/framework-integ:       
@aws-cdk-testing/framework-integ:       
@aws-cdk-testing/framework-integ:   CHANGED    aws-dynamodb/test/integ.global 2.561s
@aws-cdk-testing/framework-integ:       Resources
@aws-cdk-testing/framework-integ:       [~] AWS::Lambda::Function OnEventHandler42BEBAE0
@aws-cdk-testing/framework-integ:        └─ [~] Code
@aws-cdk-testing/framework-integ:            └─ [~] .S3Key:
@aws-cdk-testing/framework-integ:                ├─ [-] 7e256f5e4a92713cc232cd23819cb666756ee6023ce380130abac3b6a3fcea6a.zip
@aws-cdk-testing/framework-integ:                └─ [+] 7ecc77636deefb31954ea2e6aadf6d2c361a2943535a678d65f46a8de5e1f503.zip
@aws-cdk-testing/framework-integ:       [~] AWS::Lambda::Function IsCompleteHandler7073F4DA
@aws-cdk-testing/framework-integ:        └─ [~] Code
@aws-cdk-testing/framework-integ:            └─ [~] .S3Key:
@aws-cdk-testing/framework-integ:                ├─ [-] 7e256f5e4a92713cc232cd23819cb666756ee6023ce380130abac3b6a3fcea6a.zip
@aws-cdk-testing/framework-integ:                └─ [+] 7ecc77636deefb31954ea2e6aadf6d2c361a2943535a678d65f46a8de5e1f503.zip

rantoniuk · 2025-08-21T12:38:11Z

I don't think that's anyhow related to my change.

Abogical

Thank you!

This comes from an old build workflow.

mergify · 2025-08-21T13:49:41Z

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).

aws-cdk-automation

(This review is outdated)

mergify · 2025-08-21T13:51:11Z

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

mergify · 2025-08-21T14:23:07Z

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).

github-actions · 2025-08-21T14:23:23Z

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.

Copilot AI review requested due to automatic review settings July 27, 2025 19:18

Copilot AI reviewed Jul 27, 2025

View reviewed changes

github-actions bot added bug This issue is a bug. effort/medium Medium work item – several days of effort p2 beginning-contributor [Pilot] contributed between 0-2 PRs to the CDK labels Jul 27, 2025

aws-cdk-automation requested a review from a team July 27, 2025 19:18

aws-cdk-automation previously requested changes Jul 27, 2025

View reviewed changes

aws-cdk-automation added the pr-linter/exemption-requested The contributor has requested an exemption to the PR Linter feedback. label Jul 27, 2025

rantoniuk marked this pull request as draft July 28, 2025 08:23

rantoniuk force-pushed the fix/inferenceAccellerators-warning-fix branch from 594ce5e to bf6ef59 Compare August 14, 2025 09:06

github-actions bot added p1 and removed p2 labels Aug 14, 2025

rantoniuk changed the title ~~fix: remove lazy loading for deprecated inferenceAccelerators property~~ fix: stop passing the deprecated property to Cfn construct Aug 14, 2025

rantoniuk marked this pull request as ready for review August 14, 2025 09:22

rantoniuk requested a review from aws-cdk-automation August 14, 2025 10:17

fix: don't pass depreacated property to Cfn when undefined

baaeea7

rantoniuk force-pushed the fix/inferenceAccellerators-warning-fix branch from bf6ef59 to baaeea7 Compare August 20, 2025 10:04

Abogical self-assigned this Aug 20, 2025

Abogical requested changes Aug 20, 2025

View reviewed changes

packages/aws-cdk-lib/aws-ecs/lib/base/task-definition.ts Show resolved Hide resolved

rantoniuk requested a review from Abogical August 21, 2025 09:14

Abogical requested changes Aug 21, 2025

View reviewed changes

rantoniuk force-pushed the fix/inferenceAccellerators-warning-fix branch from c96a66f to 20b682f Compare August 21, 2025 12:05

chore: assertion to check for absence of InferenceAccelerators

edc8633

rantoniuk force-pushed the fix/inferenceAccellerators-warning-fix branch from 20b682f to edc8633 Compare August 21, 2025 12:06

chore: lint fix

7648323

rantoniuk requested a review from Abogical August 21, 2025 12:22

Abogical approved these changes Aug 21, 2025

View reviewed changes

Abogical requested changes Aug 21, 2025

View reviewed changes

Merge branch 'main' into fix/inferenceAccellerators-warning-fix

2d3ea69

rantoniuk requested a review from Abogical August 21, 2025 12:42

Abogical approved these changes Aug 21, 2025

View reviewed changes

Merge branch 'main' into fix/inferenceAccellerators-warning-fix

fdc8535

aws-cdk-automation previously requested changes Aug 21, 2025

View reviewed changes

Abogical added the pr-linter/exempt-integ-test The PR linter will not require integ test changes label Aug 21, 2025

mergify bot merged commit a20afcd into aws:main Aug 21, 2025
27 checks passed

github-actions bot locked as resolved and limited conversation to collaborators Aug 21, 2025

Conversation

rantoniuk commented Jul 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Change

Tests

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

aws-cdk-automation left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rantoniuk commented Jul 27, 2025

Uh oh!

aws-cdk-automation commented Jul 27, 2025

AWS CodeBuild CI Report

Uh oh!

rantoniuk commented Aug 18, 2025

Uh oh!

Abogical left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rantoniuk commented Aug 21, 2025

Uh oh!

Abogical commented Aug 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Abogical left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Abogical left a comment

Choose a reason for hiding this comment

Uh oh!

rantoniuk commented Aug 21, 2025

Uh oh!

Abogical left a comment

Choose a reason for hiding this comment

Uh oh!

mergify bot commented Aug 21, 2025

Uh oh!

aws-cdk-automation left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mergify bot commented Aug 21, 2025

Uh oh!

mergify bot commented Aug 21, 2025

Uh oh!

Uh oh!

github-actions bot commented Aug 21, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

rantoniuk commented Jul 27, 2025 •

edited

Loading

aws-cdk-automation left a comment •

edited

Loading

Abogical commented Aug 21, 2025 •

edited

Loading

Abogical left a comment •

edited

Loading

aws-cdk-automation left a comment •

edited

Loading