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.

Sign up for GitHub

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

Jump to bottom

feat(new construct): aws-fargate-kinesisstreams #877

Merged
merged 13 commits into from
Dec 23, 2022
Merged

feat(new construct): aws-fargate-kinesisstreams #877

merged 13 commits into from
Dec 23, 2022

Conversation

georgebearden
Copy link
Contributor

Issue #, if available:

README and architecture diagram for the new aws-fargate-kinesisstreams solutions construct.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@georgebearden georgebearden self-assigned this Dec 16, 2022
@aws-solutions-constructs-team
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: githubautobuild-for-cdk-v2
  • Commit ID: ae3b069
  • Result: FAILED
  • Build Logs (available for 30 days)

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

@aws-solutions-constructs-team
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: githubautobuild-for-cdk-v2
  • Commit ID: e13b2d4
  • Result: FAILED
  • Build Logs (available for 30 days)

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

@aws-solutions-constructs-team
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: githubautobuild-for-cdk-v2
  • Commit ID: b7ff22c
  • Result: FAILED
  • Build Logs (available for 30 days)

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

georgebearden
georgebearden commented Dec 19, 2022
View reviewed changes
Copy link
Contributor Author

@georgebearden georgebearden left a comment

Choose a reason for hiding this comment

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

On the architecture diagram - since VPC is a required component, should we include it? I didn't see it in other fargate-based construct architecture diagrams.

@georgebearden georgebearden removed their assignment Dec 19, 2022
@aws-solutions-constructs-team
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: githubautobuild-for-cdk-v2
  • Commit ID: b382007
  • Result: FAILED
  • Build Logs (available for 30 days)

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

@aws-solutions-constructs-team
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: githubautobuild-for-cdk-v2
  • Commit ID: ff3b846
  • Result: FAILED
  • Build Logs (available for 30 days)

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

biffgaut and others added 6 commits December 23, 2022 14:11
@biffgaut @georgebearden
feat(new construct): aws-lambda-kinesisfirehose (#875)
42146e4 
* Initial README.md draft

* Minor edits, arch diagram

* Typo on arch diagram

* First draft - complete implementation

* Add dependency to package.json

* Suppress autodelete function cfn-nag messages

* Updated snapshots, self-review updates

* More snapshot updates

* One more cfn-nag issue

* Response to code review

* Address review

* Copyright notice updates

* Add region to stream name test change

* Fix typo

* Removing region token from generateName()

* Snapshot updates
@georgebearden
Add base code files for aws-fargate-kinesisstreams construct.
d765ee0
@georgebearden
Working on tests for the aws-fargate-kinesisstreams construct.
13cd009
@georgebearden
Update copyright date in new aws-fargate-kinesisstreams construct.
2a66c28
@georgebearden
Update integ test snapshots for aws-fargate-kinesisstreams construct.
4e5d05d
@georgebearden
Merge branch 'main' into aws-fargate-kinesisstreams
b8067fc
@aws-solutions-constructs-team
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: githubautobuild-for-cdk-v2
  • Commit ID: 4e5d05d
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@aws-solutions-constructs-team
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: githubautobuild-for-cdk-v2
  • Commit ID: b8067fc
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

biffgaut
biffgaut requested changes Dec 23, 2022
View reviewed changes
source/patterns/@aws-solutions-constructs/aws-fargate-kinesisstreams/lib/index.ts Outdated Show resolved Hide resolved
...s/@aws-solutions-constructs/aws-fargate-kinesisstreams/test/integ.fargateServiceFromProps.ts Show resolved Hide resolved
...rns/@aws-solutions-constructs/aws-fargate-kinesisstreams/test/fargate-kinesisstreams.test.ts
Tags: [
{
Key: Match.exact("Name"),
Value: Match.exact("Default/Vpc/IsolatedSubnet1")
Copy link
Contributor

Choose a reason for hiding this comment

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

Doest this create a dependency on the default VPC? Because millions of accounts don't have a default VPC (mine, for instance)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The Tag name value is a little misleading. This is not the account's default VPC, but instead a new VPC created from the ec2.Vpc construct, that has a set of default subnets in it.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK

@aws-solutions-constructs-team
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: githubautobuild-for-cdk-v2
  • Commit ID: 0e5a01d
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

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

@biffgaut biffgaut merged commit 08b7975 into main Dec 23, 2022
@biffgaut biffgaut deleted the aws-fargate-kinesisstreams branch December 23, 2022 20:48
@georgebearden georgebearden mentioned this pull request Dec 28, 2022
Closed
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@biffgaut biffgaut biffgaut approved these changes

@hayesry hayesry Awaiting requested review from hayesry hayesry is a code owner

@mobri2a mobri2a Awaiting requested review from mobri2a mobri2a is a code owner

@emcfins emcfins Awaiting requested review from emcfins emcfins is a code owner

@mickychetta mickychetta Awaiting requested review from mickychetta

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@georgebearden @aws-solutions-constructs-team @biffgaut