-
Couldn't load subscription status.
- Fork 4.3k
fix(s3-deployment): intermittent CreateInvalidation and/or wait errors #34859
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.
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(This review is outdated)
|
Exemption Request. No test available to test against an intermittent external issue. |
a7e3cd5 to
543492b
Compare
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
a595b05 to
59b04ee
Compare
59b04ee to
7c697ec
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(This review is outdated)
7c697ec to
2d88e0d
Compare
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
2d88e0d to
f70a0c3
Compare
...s/@aws-cdk/custom-resource-handlers/lib/aws-s3-deployment/bucket-deployment-handler/index.py
Show resolved
Hide resolved
0282e98 to
15cef9d
Compare
- Waiting for cache invalidation is redundant as it doesn't tell you if a cache invalidation fails anyway. Waiting is removed. - Modified retry strategy of cloudfront client to help with intermittent CreateInvalidation errors. Fixes #15891
15cef9d to
88e736f
Compare
| @@ -233,11 +239,11 @@ def cloudfront_invalidate(distribution_id, distribution_paths): | |||
| }, | |||
| 'CallerReference': str(uuid4()), | |||
| }) | |||
| # by default, will wait up to 10 minutes | |||
| cloudfront.get_waiter('invalidation_completed').wait( | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suggest we expand this to up to 13 min (instead of the default 10min it has) and then we explain in the PR itself that there is a limitation of lambda running for max 15 min, so the additional time is to allow other processing to complete. Sadly the P1 issue cannot be fully addressed due to this limitation, and since CloudFormation doesnt provide a SLA for when the invalidation will be completed after requesting, we need to keep this check to provide the users back a guarantee that the change they intended to do is either complete (success deployment), or failed to complete in the allowed time. We can potentially also improve the error message if that happens. And we also need to add better documentation to this custom resource explaining that limitation and the restrictions we have at the moment, that prevent us from fully solving the P1 issue
|
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). |
|
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). |
|
Comments on closed issues and PRs are hard for our team to see. |
Issue #15891
Closes #15891.
Reason for this change
To fix intermittent CreatInvalidation errors and wait errors when attempting to invalidating to invalidating the Cloudfront cache.
Description of changes
Describe any new or updated permissions being added
No permissions added.
Description of how you validated changes
Ran
/packages/@aws-cdk/custom-resource-handlers/test/aws-s3-deployment/bucket-deployment-handler/test.shsuccessfuly.Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license