feat(custom-resource): add serviceTimeout property for custom resources #30911

mazyu36 · 2024-07-20T17:11:51Z

Issue # (if applicable)

Reason for this change

L2 construct does not support setting serviceTimeout.

Enabling customizable timeouts is useful when using custom resources, as the current default timeout is set to 3600 seconds.

Ref: AWS CloudFormation accelerates dev-test cycle with adjustable timeouts for custom resources

Description of changes

Add serviceTimeout property for CustomResource and AwsCustomResource.

Description of how you validated changes

Add unit tests and integ tests.
Additionally, I confirmed that ServiceTimeout is set by checking the CloudWatch Logs of the Lambda function that generates custom resources.

Checklist

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

xazhao

Left a few minor comments. The code look good to me overall.

xazhao · 2024-07-30T17:31:45Z

packages/aws-cdk-lib/core/lib/custom-resource.ts

@@ -55,6 +56,13 @@ export interface CustomResourceProps {
   */
  readonly serviceToken: string;

+  /**
+   * The maximum time, in seconds, that can elapse before a custom resource operation times out


Do we need to mention the unit is seconds here? Since the type is Duration, customers should be able to use different units in Duration.
Also might be good to mention the range is 1 to 3600 seconds here.

@xazhao
Thanks.
I've updated docs.

xazhao · 2024-07-30T17:44:13Z

.../framework-integ/test/aws-cloudformation/test/integ.core-custom-resources-service-timeout.ts

+/*
+ * Stack verification steps:
+ * - Deploy with `--no-clean`
+ * - Verify that `ServiceTimeout` is set to 60 in the CloudWatch Logs for the Lambda function that creates custom resources.


Curious how did you verify the ServiceTimeout is set to 60?

Nvm I see you explained it in the PR description.

TheRealAmazonKendra · 2024-07-31T18:01:31Z

I don't think we can accept this PR at this time, not because of anything wrong with your code but because of problems with AwsCustomResource. I'm going to put the do-not-close and do-not-merge label on this one while we work to assess whether this is actually safe to accept and/or resolve the other issues at play here.

polothy · 2024-10-31T17:00:26Z

Ran into wanting this feature (currently waiting for things to timeout hah), what's the concern with AwsCustomResource? The default is still an hour as it always was?

mirodrr2 · 2024-11-19T19:03:45Z

Please prioritize merging this, it's a huge pain point when using Custom Resources in cdk

If the custom resource has an unhandled exception, you have no choice but to wait around for an hour

GavinZZ · 2024-11-20T22:25:00Z

Hello, thanks for pinging us on this issue. I apologize for the delay on the update on this issue. I will sync with Kendra internally and share some update on this PR.

codecov · 2024-11-23T00:31:24Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 66.96%. Comparing base (a10c369) to head (72a269f).

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #30911   +/-   ##
=======================================
  Coverage   66.96%   66.96%           
=======================================
  Files         329      329           
  Lines       18663    18667    +4     
  Branches     3258     3260    +2     
=======================================
+ Hits        12497    12501    +4     
  Misses       5839     5839           
  Partials      327      327

Flag	Coverage Δ
suite.unit	`66.96% <100.00%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
packages/aws-cdk	`80.71% <ø> (ø)`
packages/aws-cdk-lib/core	`82.09% <100.00%> (+0.01%)`	⬆️

GavinZZ · 2024-11-25T22:33:15Z

Hello, thanks for pinging us on this issue. I apologize for the delay on the update on this issue. I will sync with Kendra internally and share some update on this PR.

Kendra is currently out of office, and I haven’t had a chance to hear back from her yet. I’d like to understand her concerns first before proceeding with the PR review. Thank you for your patience!

mergify · 2024-12-18T21:32:38Z

update

✅ Branch has been successfully updated

Pull request has been modified.

mirodrr2 · 2025-01-02T19:23:17Z

@GavinZZ , any updates on this?

xazhao · 2025-01-03T18:57:32Z

@mirodrr2 this PR is ready to merge. I will approve and get it merged.

Pull request has been modified.

xazhao · 2025-01-03T23:41:52Z

@Mergifyio update

mergify · 2025-01-03T23:42:06Z

update

✅ Branch has been successfully updated

mazyu36 · 2025-01-04T04:49:53Z

@xazhao
Thank you for the approval.
Would you please remove the pr/do-not-merge label?
This label prevents the merge process from proceeding, I think.

mergify · 2025-01-04T05:36:27Z

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 · 2025-01-04T06:09:16Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
Commit ID: 72a269f
Result: SUCCEEDED
Build Logs (available for 30 days)

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

mergify · 2025-01-04T06:09:38Z

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-01-04T06:09:59Z

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.

feat: add serviceTimeout property for custom resources

45f6229

aws-cdk-automation requested a review from a team July 20, 2024 17:12

github-actions bot added effort/medium Medium work item – several days of effort feature-request A feature should be added or improved. p1 star-contributor [Pilot] contributed between 25-49 PRs to the CDK labels Jul 20, 2024

Merge branch 'main' into custom-resource-service-timeout-30517

4794871

aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Jul 20, 2024

github-actions bot mentioned this pull request Jul 22, 2024

Weekly PR metrics report #30917

Closed

xazhao self-assigned this Jul 30, 2024

xazhao reviewed Jul 30, 2024

View reviewed changes

update docs

d29130b

mazyu36 force-pushed the custom-resource-service-timeout-30517 branch from df369dc to d29130b Compare July 30, 2024 23:16

mazyu36 requested a review from xazhao July 31, 2024 00:01

TheRealAmazonKendra added pr/do-not-merge This PR should not be merged at this time. pr-linter/do-not-close The PR linter will not close this PR while this label is present labels Jul 31, 2024

github-actions bot mentioned this pull request Aug 1, 2024

Monthly PR metrics report #30999

Closed

mazyu36 added 4 commits November 1, 2024 09:28

Merge branch 'main' into custom-resource-service-timeout-30517

5f095b2

update integ test

430a718

Merge branch 'main' into custom-resource-service-timeout-30517

11c0e79

Merge branch 'main' into custom-resource-service-timeout-30517

41692dc

pahud mentioned this pull request Nov 22, 2024

custom-resource: Support of ServiceTimeout from Cloudformation #30517

Closed

2 tasks

Merge branch 'main' into custom-resource-service-timeout-30517

4eb9931

mazyu36 requested a review from a team as a code owner November 23, 2024 00:18

Merge branch 'main' into custom-resource-service-timeout-30517

9ab4736

aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Dec 18, 2024

Merge branch 'main' into custom-resource-service-timeout-30517

aefffb1

aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Dec 19, 2024

Merge branch 'main' into custom-resource-service-timeout-30517

865ff06

xazhao previously approved these changes Jan 3, 2025

View reviewed changes

aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Jan 3, 2025

Merge branch 'main' into custom-resource-service-timeout-30517

403ae5b

aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Jan 3, 2025

xazhao approved these changes Jan 3, 2025

View reviewed changes

aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Jan 3, 2025

Merge branch 'main' into custom-resource-service-timeout-30517

848e854

xazhao removed pr/do-not-merge This PR should not be merged at this time. pr-linter/do-not-close The PR linter will not close this PR while this label is present labels Jan 4, 2025

Merge branch 'main' into custom-resource-service-timeout-30517

72a269f

mergify bot merged commit d599900 into aws:main Jan 4, 2025
19 checks passed

github-actions bot locked as resolved and limited conversation to collaborators Jan 4, 2025

mazyu36 deleted the custom-resource-service-timeout-30517 branch January 4, 2025 06:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(custom-resource): add serviceTimeout property for custom resources #30911

feat(custom-resource): add serviceTimeout property for custom resources #30911

mazyu36 commented Jul 20, 2024

xazhao left a comment

xazhao Jul 30, 2024

mazyu36 Jul 31, 2024

xazhao Jul 30, 2024

xazhao Jul 30, 2024

TheRealAmazonKendra commented Jul 31, 2024

polothy commented Oct 31, 2024

mirodrr2 commented Nov 19, 2024 •

edited

Loading

GavinZZ commented Nov 20, 2024

codecov bot commented Nov 23, 2024 •

edited

Loading

GavinZZ commented Nov 25, 2024

mergify bot commented Dec 18, 2024

mirodrr2 commented Jan 2, 2025

xazhao commented Jan 3, 2025

xazhao commented Jan 3, 2025

mergify bot commented Jan 3, 2025

mazyu36 commented Jan 4, 2025

mergify bot commented Jan 4, 2025

aws-cdk-automation commented Jan 4, 2025

mergify bot commented Jan 4, 2025

github-actions bot commented Jan 4, 2025

feat(custom-resource): add serviceTimeout property for custom resources #30911

feat(custom-resource): add serviceTimeout property for custom resources #30911

Conversation

mazyu36 commented Jul 20, 2024

Issue # (if applicable)

Reason for this change

Description of changes

Description of how you validated changes

Checklist

xazhao left a comment

Choose a reason for hiding this comment

xazhao Jul 30, 2024

Choose a reason for hiding this comment

mazyu36 Jul 31, 2024

Choose a reason for hiding this comment

xazhao Jul 30, 2024

Choose a reason for hiding this comment

xazhao Jul 30, 2024

Choose a reason for hiding this comment

TheRealAmazonKendra commented Jul 31, 2024

polothy commented Oct 31, 2024

mirodrr2 commented Nov 19, 2024 • edited Loading

GavinZZ commented Nov 20, 2024

codecov bot commented Nov 23, 2024 • edited Loading

Codecov Report

GavinZZ commented Nov 25, 2024

mergify bot commented Dec 18, 2024

✅ Branch has been successfully updated

mirodrr2 commented Jan 2, 2025

xazhao commented Jan 3, 2025

xazhao commented Jan 3, 2025

mergify bot commented Jan 3, 2025

✅ Branch has been successfully updated

mazyu36 commented Jan 4, 2025

mergify bot commented Jan 4, 2025

aws-cdk-automation commented Jan 4, 2025

AWS CodeBuild CI Report

mergify bot commented Jan 4, 2025

github-actions bot commented Jan 4, 2025

mirodrr2 commented Nov 19, 2024 •

edited

Loading

codecov bot commented Nov 23, 2024 •

edited

Loading