fix: check if inferenceAccellerators has any elements

[rantoniuk]

Issue # (if applicable)

Closes #33505

Reason for this change

See this comment.

Description of changes

Do not rely only on passing the props, but check if the array actually contains any elements.

Describe any new or updated permissions being added

Description of how you validated changes

Cannot test by hand due to #34610. UPDATE: manually tested in a production CDK stack. Result as expected, warnings are not shown during cdk diff.

Checklist

• [x] My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

---

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

[rantoniuk]

Clarification Request: I don't see any case like this being tested in packages/aws-cdk-lib/aws-ecs/test/task-definition.test.ts and I think this is a cosmetic change to how the condition is evaluated.

I'm happy to add a test if required but please suggest the correct place and approach.

[rantoniuk]

@leonmk-aws any thoughts?

[leonmk-aws]

Thank you for your contribution, I have looked at this PR, unfortunately I don't believe it will close the issue that is linked. Here is my reasoning: The value of the optional list of inference accelerators props.inferenceAccelerator is undefined when not specified, which means that the addInferenceAccelerator is never called because of the conditional check: https://github.com/aws/aws-cdk/blob/89d2d5c561860c1a9b55e1660efeec068fc4e6a3/packages/aws-cdk-lib/aws-ecs/lib/base/task-definition.ts#L492-L494

I also tried to see if the fix was working by pulling your change in my repo, and using the the reproduction stack posted in the issue thread, I had the same warning without and with the changes. I am not sure how you validated that the changes were working in your setup, here is how I did it:

• First compile the repo without your changes, use the linker script to link it to a fresh cdk app containing the reproduction stack linked above, run cdk list (or any cdk command like cdk synth, note that you don't need to deploy the stack for the warning to appear), the warning should be printed in the console.
• In a second time, make your changes to the repo and rebuild it, in the same cdk app you used in step 1, re-run the same command, if the changes are working the warning should have disappeared.

[github-actions[bot]]

This PR has not received a response in a while. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.

[rantoniuk]

Hold your horses gh-bot, I will take a look at this this week

[rantoniuk]

I can confirm I can reproduce it using the small stack but I'm sure that it worked for my prod stack (but it doesn't now), very weird.

I see one big difference between the two, ES2022 vs ES2020 so I had to juggle the tsconfig settings a bit. I'll try to find the root cause, appreciate it!

[rantoniuk]

@leonmk-aws that should do it - I checked this with both, the sample app and my prod stack.

[rantoniuk]

I see some checks have failed but at first glance it doesn't look they are related to my change?

[rantoniuk]

@leonmk-aws is it still waiting for Exemption Request?

[mergify[bot]]

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]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
• Commit ID: 4a546fb97f133054282e1a1cea0ad5e7d0f0faab
• Result: SUCCEEDED
• Build Logs (available for 30 days)

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

[mergify[bot]]

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[bot]]

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.