Skip to content
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(kinesisfirehose): supports Kinesis data stream source for delivery stream #15836

Merged
merged 223 commits into from
Aug 4, 2021

Conversation

BenChaimberg
Copy link
Contributor

@BenChaimberg BenChaimberg commented Jul 30, 2021

closes #15500
closes #10783


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

madeline-k and others added 30 commits July 14, 2021 10:04
…eam (#15545)

Closes #15543 

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@gitpod-io
Copy link

gitpod-io bot commented Jul 30, 2021

@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Jul 30, 2021
@BenChaimberg BenChaimberg requested a review from madeline-k July 30, 2021 06:00
Base automatically changed from madeline-k/feat/kinesisfirehose-backup to master July 30, 2021 18:22
Copy link
Contributor

@madeline-k madeline-k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great! A couple minor comments.

Comment on lines +21 to +32
const mockS3Destination: firehose.IDestination = {
bind(_scope: constructs.Construct, _options: firehose.DestinationBindOptions): firehose.DestinationConfig {
const bucketGrant = bucket.grantReadWrite(role);
return {
extendedS3DestinationConfiguration: {
bucketArn: bucket.bucketArn,
roleArn: role.roleArn,
},
dependables: [bucketGrant],
};
},
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this duplicated with the other integration test? Can you make one shared construct for this for the unit and integ tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think for literacy purposes, I'd rather keep this duplicated

@BenChaimberg BenChaimberg requested review from madeline-k and a team August 3, 2021 19:35
Copy link
Contributor

@madeline-k madeline-k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great 👍

@mergify
Copy link
Contributor

mergify bot commented Aug 4, 2021

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).

@mergify mergify bot merged commit afd5bf7 into master Aug 4, 2021
@mergify mergify bot deleted the chaimber/firehose-kinesis-source branch August 4, 2021 20:13
@mergify
Copy link
Contributor

mergify bot commented Aug 4, 2021

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).

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
  • Commit ID: 98c77bf
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

hollanddd pushed a commit to hollanddd/aws-cdk that referenced this pull request Aug 26, 2021
…ry stream (aws#15836)

closes aws#15500 
closes aws#10783 

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
david-doyle-as24 pushed a commit to david-doyle-as24/aws-cdk that referenced this pull request Sep 7, 2021
…ry stream (aws#15836)

closes aws#15500 
closes aws#10783 

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
contribution/core This is a PR that came from AWS.
Projects
None yet
3 participants