-
Notifications
You must be signed in to change notification settings - Fork 249
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
feat(aws-s3-stepfunctions): Changed escape hatch to eventBridgeEnabled prop #666
Conversation
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
const cfnBucket = existingBucket.node.defaultChild as s3.CfnBucket; | ||
cfnBucket.addPropertyOverride('NotificationConfiguration.EventBridgeConfiguration.EventBridgeEnabled', true); | ||
const existingBucket = CreateScrapBucket(stack, { | ||
eventBridgeEnabled: true |
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.
Is there now a requirement that if an existing bucket is provided that eventBridgeEnabled is set to true? We should document that in the README. Better yet, can we turn it on ourselves? (I'm guessing that if this accepts an IBucket then we are unable to do so...)
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.
Correct, we are unable to turn it on ourselves
source/patterns/@aws-solutions-constructs/aws-s3-stepfunctions/lib/index.ts
Outdated
Show resolved
Hide resolved
@@ -34,7 +34,7 @@ export interface S3ToStepfunctionsProps { | |||
/** | |||
* Optional user provided props to override the default props for the S3 Bucket. |
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.
Here's where we need a comment that the events feature must already be turned on for an existing bucket as this construct can't do it from an IBucket (I assume...)
And in the README.md
source/patterns/@aws-solutions-constructs/aws-s3-stepfunctions/lib/index.ts
Outdated
Show resolved
Hide resolved
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Issue #565 , if available:
Description of changes:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
fixes #565