-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
fix(scheduler-targets-alpha): kinesis data firehose target uses l1 instead of l2 #32150
Conversation
|
||
beforeEach(() => { | ||
app = new App({ context: { '@aws-cdk/aws-iam:minimizePolicies': true } }); | ||
stack = new Stack(app, 'Stack', { env: { region: 'us-east-1', account: '123456789012' } }); | ||
firehose = new CfnDeliveryStream(stack, 'MyFirehose'); | ||
mockS3Destination = { |
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 test setup was taken from https://github.com/aws/aws-cdk/blob/main/packages/%40aws-cdk/aws-kinesisfirehose-alpha/test/delivery-stream.test.ts
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #32150 +/- ##
=======================================
Coverage 77.18% 77.18%
=======================================
Files 105 105
Lines 7163 7163
Branches 1311 1311
=======================================
Hits 5529 5529
Misses 1454 1454
Partials 180 180
Flags with carried forward coverage won't be shown. Click here to find out more.
|
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). |
@Mergifyio update |
☑️ Nothing to do
|
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 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 # (if applicable)
Tracking #31785
Reason for this change
Since the Kinesis Data Firehose Alpha module is in developer preview and contains the L2 construct for a Firehose Delivery Stream, we should make this upgrade from L1 to L2 now while the module is experimental instead of in the future, where we would need to add a V2 for this target (like for event targets, see #30189) to avoid breaking customers.
Description of changes
Replace CfnDeliveryStream with IDeliveryStream. The L1 uses
S3DestinationConfiguration
property whereas the L2 usesExtendedS3DestinationConfiguration
property (includes additional fields on top of S3DestinationConfiguration fields). According tohttps://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-kinesisfirehose-deliverystream.html, "If you change the delivery stream destination from an Amazon S3 destination to an Amazon ES destination, update requires some interruptions."
Description of how you validated changes
Checklist
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license
BREAKING CHANGE: KinesisDataFirehosePutRecord scheduler target now accepts IDeliveryStream instead of CfnDeliveryStream.