-
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-fargate-stepfunctions): new construct #677
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 |
source/patterns/@aws-solutions-constructs/aws-fargate-stepfunctions/README.md
Outdated
Show resolved
Hide resolved
source/patterns/@aws-solutions-constructs/aws-fargate-stepfunctions/README.md
Outdated
Show resolved
Hide resolved
source/patterns/@aws-solutions-constructs/aws-fargate-stepfunctions/README.md
Outdated
Show resolved
Hide resolved
source/patterns/@aws-solutions-constructs/aws-fargate-stepfunctions/README.md
Outdated
Show resolved
Hide resolved
source/patterns/@aws-solutions-constructs/aws-fargate-stepfunctions/README.md
Outdated
Show resolved
Hide resolved
source/patterns/@aws-solutions-constructs/aws-fargate-stepfunctions/README.md
Outdated
Show resolved
Hide resolved
source/patterns/@aws-solutions-constructs/aws-fargate-stepfunctions/README.md
Show resolved
Hide resolved
source/patterns/@aws-solutions-constructs/aws-fargate-stepfunctions/README.md
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 |
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 |
source/patterns/@aws-solutions-constructs/aws-fargate-stepfunctions/lib/index.ts
Outdated
Show resolved
Hide resolved
expect(stack).toCountResources('AWS::ECS::Service', 1); | ||
} | ||
|
||
function checkForBaseResources(stack: cdk.Stack) { |
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.
Not a fan of this - makes it difficult to see what each test is looking for. Let's not do this here, and going forward - our new style of checking 1 thing in each test should eliminate our need to have lots of checks duplicated between tests.
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.
If you'd rather change to the new testing style here rather than replacing all these statements, that's fine (although I think replacing the statements would be quicker, even if it annoys... :-) )
return defaults.consolidateProps(defaultTestProp, userProps); | ||
} | ||
|
||
function serviceTest() { |
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.
Saw all these after i saw CheckForBaseResources() below. Same principle applies - if we don't want to replicate this extensive code/json, then let's rearrange the tests so it only appears once. For these especially, at some point a change in a single test will cause that test to start to fail and we'll have to unwind this change for that test.
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 |
expect(stack).toHaveResourceLike("AWS::Logs::LogGroup", { | ||
LogGroupName: logGroupName | ||
}); | ||
}); | ||
|
||
function createFargateConstructWithNewResources(stack: cdk.Stack, publicApi: boolean) { | ||
return new FargateToStepfunctions(stack, 'test-construct', { |
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 is the actual setup that's being tested - I don't think saving 10 lines of code in a test at the expense of hiding the the target of the test is a good tradeoff.
Let's leave it like this for now - but let's discuss the idea with the Clean Code group next week to see what a larger audience thinks going forward.
containerDefinitionProps: { containerName }, | ||
fargateTaskDefinitionProps: { family: familyName }, | ||
fargateServiceProps: { serviceName }, | ||
stateMachineProps: testStateMachineProps(stack), |
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 I like (testStateMachineProps).
* chore(release): 2.7.0 * chore(changelog): Updated CHANGELOG.v2.md * feat(aws-fargate-stepfunctions): new construct (#677) * created README.md * removed arn env var prop and fixed state machine doc comments * made createCloudWatchAlarms prop optional * created new construct * fixed README typo * refactored test to single concept design * updated default for vpcProps * Update copyright year to 2022 (#693) Skipping review, only copyright messages have been changed. * fix(Test Coverage): Improve test coverage of 2 core files (#691) * Added additional tests * More coverage based on review * chore(Improve documentation): Environment Variable descriptions (#692) * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update Environment Variable documenation/comments * Results of self-review * Response to review * Review 2 * Review 3 * Update DESIGN_GUIDELINES.md * Pin @types/prettier 2.6.0 til 2.6.1 issue resolved (#698) No one is available to review and this is time sensitive. The nature of the change makes any review rather perfunctory as it a single line added to the package.json file * chore(release): 1.155.0 * chore(changelog): Updated CHANGELOG.md * chore(changelog): Updated CHANGELOG.md * Refresh to accomodate CDK changes * Align CDK V2 version * Align CDK Versions * Align CDK Versions Co-authored-by: biffgaut <78155736+biffgaut@users.noreply.github.com> Co-authored-by: mickychetta <45010053+mickychetta@users.noreply.github.com>
* chore(release): 2.7.0 * chore(changelog): Updated CHANGELOG.v2.md * feat(aws-fargate-stepfunctions): new construct (#677) * created README.md * removed arn env var prop and fixed state machine doc comments * made createCloudWatchAlarms prop optional * created new construct * fixed README typo * refactored test to single concept design * updated default for vpcProps * Update copyright year to 2022 (#693) Skipping review, only copyright messages have been changed. * fix(Test Coverage): Improve test coverage of 2 core files (#691) * Added additional tests * More coverage based on review * chore(Improve documentation): Environment Variable descriptions (#692) * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update Environment Variable documenation/comments * Results of self-review * Response to review * Review 2 * Review 3 * Update DESIGN_GUIDELINES.md * Pin @types/prettier 2.6.0 til 2.6.1 issue resolved (#698) No one is available to review and this is time sensitive. The nature of the change makes any review rather perfunctory as it a single line added to the package.json file * chore(release): 1.155.0 * chore(changelog): Updated CHANGELOG.md * chore(changelog): Updated CHANGELOG.md * Refresh to accomodate CDK changes * Align CDK V2 version * Align CDK Versions * Align CDK Versions Co-authored-by: biffgaut <78155736+biffgaut@users.noreply.github.com> Co-authored-by: mickychetta <45010053+mickychetta@users.noreply.github.com> Co-authored-by: biffgaut <78155736+biffgaut@users.noreply.github.com> Co-authored-by: mickychetta <45010053+mickychetta@users.noreply.github.com>
* chore(release): 2.7.0 * chore(changelog): Updated CHANGELOG.v2.md * feat(aws-fargate-stepfunctions): new construct (#677) * created README.md * removed arn env var prop and fixed state machine doc comments * made createCloudWatchAlarms prop optional * created new construct * fixed README typo * refactored test to single concept design * updated default for vpcProps * Update copyright year to 2022 (#693) Skipping review, only copyright messages have been changed. * fix(Test Coverage): Improve test coverage of 2 core files (#691) * Added additional tests * More coverage based on review * chore(Improve documentation): Environment Variable descriptions (#692) * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update Environment Variable documenation/comments * Results of self-review * Response to review * Review 2 * Review 3 * Update DESIGN_GUIDELINES.md * Pin @types/prettier 2.6.0 til 2.6.1 issue resolved (#698) No one is available to review and this is time sensitive. The nature of the change makes any review rather perfunctory as it a single line added to the package.json file * chore(release): 1.155.0 (#699) (#701) * chore(release): 2.7.0 * chore(changelog): Updated CHANGELOG.v2.md * feat(aws-fargate-stepfunctions): new construct (#677) * created README.md * removed arn env var prop and fixed state machine doc comments * made createCloudWatchAlarms prop optional * created new construct * fixed README typo * refactored test to single concept design * updated default for vpcProps * Update copyright year to 2022 (#693) Skipping review, only copyright messages have been changed. * fix(Test Coverage): Improve test coverage of 2 core files (#691) * Added additional tests * More coverage based on review * chore(Improve documentation): Environment Variable descriptions (#692) * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update Environment Variable documenation/comments * Results of self-review * Response to review * Review 2 * Review 3 * Update DESIGN_GUIDELINES.md * Pin @types/prettier 2.6.0 til 2.6.1 issue resolved (#698) No one is available to review and this is time sensitive. The nature of the change makes any review rather perfunctory as it a single line added to the package.json file * chore(release): 1.155.0 * chore(changelog): Updated CHANGELOG.md * chore(changelog): Updated CHANGELOG.md * Refresh to accomodate CDK changes * Align CDK V2 version * Align CDK Versions * Align CDK Versions Co-authored-by: biffgaut <78155736+biffgaut@users.noreply.github.com> Co-authored-by: mickychetta <45010053+mickychetta@users.noreply.github.com> Co-authored-by: biffgaut <78155736+biffgaut@users.noreply.github.com> Co-authored-by: mickychetta <45010053+mickychetta@users.noreply.github.com> * Expedited update to address @types/prettier versioning issue. * chore(release): 1.156.0 * chore(changelog): Updated CHANGELOG.md Co-authored-by: biffgaut <78155736+biffgaut@users.noreply.github.com> Co-authored-by: mickychetta <45010053+mickychetta@users.noreply.github.com>
* chore(release): 1.155.0 (#699) * chore(release): 2.7.0 * chore(changelog): Updated CHANGELOG.v2.md * feat(aws-fargate-stepfunctions): new construct (#677) * created README.md * removed arn env var prop and fixed state machine doc comments * made createCloudWatchAlarms prop optional * created new construct * fixed README typo * refactored test to single concept design * updated default for vpcProps * Update copyright year to 2022 (#693) Skipping review, only copyright messages have been changed. * fix(Test Coverage): Improve test coverage of 2 core files (#691) * Added additional tests * More coverage based on review * chore(Improve documentation): Environment Variable descriptions (#692) * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update Environment Variable documenation/comments * Results of self-review * Response to review * Review 2 * Review 3 * Update DESIGN_GUIDELINES.md * Pin @types/prettier 2.6.0 til 2.6.1 issue resolved (#698) No one is available to review and this is time sensitive. The nature of the change makes any review rather perfunctory as it a single line added to the package.json file * chore(release): 1.155.0 * chore(changelog): Updated CHANGELOG.md * chore(changelog): Updated CHANGELOG.md * Refresh to accomodate CDK changes * Align CDK V2 version * Align CDK Versions * Align CDK Versions Co-authored-by: biffgaut <78155736+biffgaut@users.noreply.github.com> Co-authored-by: mickychetta <45010053+mickychetta@users.noreply.github.com> * chore(release): 1.156.0 (#704) * chore(release): 2.7.0 * chore(changelog): Updated CHANGELOG.v2.md * feat(aws-fargate-stepfunctions): new construct (#677) * created README.md * removed arn env var prop and fixed state machine doc comments * made createCloudWatchAlarms prop optional * created new construct * fixed README typo * refactored test to single concept design * updated default for vpcProps * Update copyright year to 2022 (#693) Skipping review, only copyright messages have been changed. * fix(Test Coverage): Improve test coverage of 2 core files (#691) * Added additional tests * More coverage based on review * chore(Improve documentation): Environment Variable descriptions (#692) * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update Environment Variable documenation/comments * Results of self-review * Response to review * Review 2 * Review 3 * Update DESIGN_GUIDELINES.md * Pin @types/prettier 2.6.0 til 2.6.1 issue resolved (#698) No one is available to review and this is time sensitive. The nature of the change makes any review rather perfunctory as it a single line added to the package.json file * chore(release): 1.155.0 (#699) (#701) * chore(release): 2.7.0 * chore(changelog): Updated CHANGELOG.v2.md * feat(aws-fargate-stepfunctions): new construct (#677) * created README.md * removed arn env var prop and fixed state machine doc comments * made createCloudWatchAlarms prop optional * created new construct * fixed README typo * refactored test to single concept design * updated default for vpcProps * Update copyright year to 2022 (#693) Skipping review, only copyright messages have been changed. * fix(Test Coverage): Improve test coverage of 2 core files (#691) * Added additional tests * More coverage based on review * chore(Improve documentation): Environment Variable descriptions (#692) * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update README.md * Update Environment Variable documenation/comments * Results of self-review * Response to review * Review 2 * Review 3 * Update DESIGN_GUIDELINES.md * Pin @types/prettier 2.6.0 til 2.6.1 issue resolved (#698) No one is available to review and this is time sensitive. The nature of the change makes any review rather perfunctory as it a single line added to the package.json file * chore(release): 1.155.0 * chore(changelog): Updated CHANGELOG.md * chore(changelog): Updated CHANGELOG.md * Refresh to accomodate CDK changes * Align CDK V2 version * Align CDK Versions * Align CDK Versions Co-authored-by: biffgaut <78155736+biffgaut@users.noreply.github.com> Co-authored-by: mickychetta <45010053+mickychetta@users.noreply.github.com> Co-authored-by: biffgaut <78155736+biffgaut@users.noreply.github.com> Co-authored-by: mickychetta <45010053+mickychetta@users.noreply.github.com> * Expedited update to address @types/prettier versioning issue. * chore(release): 1.156.0 * chore(changelog): Updated CHANGELOG.md Co-authored-by: biffgaut <78155736+biffgaut@users.noreply.github.com> Co-authored-by: mickychetta <45010053+mickychetta@users.noreply.github.com> Co-authored-by: biffgaut <78155736+biffgaut@users.noreply.github.com> Co-authored-by: mickychetta <45010053+mickychetta@users.noreply.github.com>
Issue #676 , if available:
Description of changes:
Should create a VPC or accept an existing VPC
Should create a Fargate Service or accept an existing service
Should create a new step function or accept an existing step function
Should grant the service IAM permission to read/write to the function
Should set up environment variables in the service that identify the function in the construct code so the container can read/write from the function
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
fixes #676