-
Notifications
You must be signed in to change notification settings - Fork 4k
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
chore: remove an additional "prepare" usage #9428
Conversation
Follow up on #9410 and remove an additional usage of `prepare` in the API Gateway library. In this case we leveraged `prepare()` to record dependencies between all Deployment resources and all Method resources in the APIGW. The solution is to perform two-sided bookkeeping while methods/deployments are defined and record their dependencies in-band. We also refactored the way the code in `LatestDeployment` to be slightly more readable.
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.
Looks ok to me but I think @nija-at should have a look as well.
Yes, no worries. I contacted him and he'll take a look tomorrow. |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
Follow up on #9410 and remove a few additional usages of `prepare`: - In the API Gateway library we leveraged `prepare()` to record dependencies between all Deployment resources and all Method resources in the APIGW. The solution is to perform two-sided bookkeeping while methods/deployments are defined and record their dependencies in-band. We also refactored the way the code in `LatestDeployment` to be slightly more readable. - In the Pipelines library prepare was replaced with an aspect (which is technically the drop-in alternative to `prepare()` in v2.0, for lack of a better solution at the moment). - In the IAM library, the `Policy` resource needs to be conditionally created only if the document contains statements. To address that, we added a new protected API to `CfnResource` which is called `shouldSynthesize()`. By default it returns `true` but if it returns `false` (in a subclass), the resource will not be rendered into the cloudformation template. Related: aws/aws-cdk-rfcs#192 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Follow up on aws#9410 and remove a few additional usages of `prepare`: - In the API Gateway library we leveraged `prepare()` to record dependencies between all Deployment resources and all Method resources in the APIGW. The solution is to perform two-sided bookkeeping while methods/deployments are defined and record their dependencies in-band. We also refactored the way the code in `LatestDeployment` to be slightly more readable. - In the Pipelines library prepare was replaced with an aspect (which is technically the drop-in alternative to `prepare()` in v2.0, for lack of a better solution at the moment). - In the IAM library, the `Policy` resource needs to be conditionally created only if the document contains statements. To address that, we added a new protected API to `CfnResource` which is called `shouldSynthesize()`. By default it returns `true` but if it returns `false` (in a subclass), the resource will not be rendered into the cloudformation template. Related: aws/aws-cdk-rfcs#192 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Follow up on #9410 and remove a few additional usages of
prepare
:prepare()
to record dependencies between all Deployment resources and all Method resources in the APIGW. The solution is to perform two-sided bookkeeping while methods/deployments are defined and record their dependencies in-band. We also refactored the way the code inLatestDeployment
to be slightly more readable.prepare()
in v2.0, for lack of a better solution at the moment).Policy
resource needs to be conditionally created only if the document contains statements. To address that, we added a new protected API toCfnResource
which is calledshouldSynthesize()
. By default it returnstrue
but if it returnsfalse
(in a subclass), the resource will not be rendered into the cloudformation template.Related: aws/aws-cdk-rfcs#192
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license