-
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(README.md): add python and java minimal deployment #582
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 |
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.
Stopped after a couple files so indents could be made consistent before I read the whole thing.
|
||
# Obtain a pre-existing certificate from your account | ||
certificate = acm.Certificate.from_certificate_arn( | ||
self, |
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 indent doesn't match the indent in the next function call (line 68). I know that indentions are important in Python ([sarcasm]one of the things I love so much about it[/sarcasm]), we should be consistent.
code: lambda.Code.fromAsset(`${__dirname}/lambda`), | ||
runtime: lambda.Runtime.NODEJS_14_X, | ||
handler: 'index.handler' | ||
code: lambda.Code.fromAsset(`${__dirname}/lambda`), |
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.
These attributes should be indented 2. The only 4 space indent is multiple arguments for a function call.
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 |
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 |
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.
Every V1 construct sample needs a comment (in the correct style for the language):
// aws-events-rule-lambda has been deprecated for CDK V2 in favor of aws-eventbridge-lambda.
// This sample uses the CDK V1 syntax
(the code block may not allow you to specify a link - if so, just leave off the link).
Beyond that, there are a few bugs in the Typescript samples mentioned.
source/patterns/@aws-solutions-constructs/aws-fargate-s3/README.md
Outdated
Show resolved
Hide resolved
source/patterns/@aws-solutions-constructs/aws-fargate-sns/README.md
Outdated
Show resolved
Hide resolved
source/patterns/@aws-solutions-constructs/aws-fargate-sqs/README.md
Outdated
Show resolved
Hide resolved
source/patterns/@aws-solutions-constructs/aws-fargate-sqs/README.md
Outdated
Show resolved
Hide resolved
source/patterns/@aws-solutions-constructs/aws-kinesisfirehose-s3-and-kinesisanalytics/README.md
Outdated
Show resolved
Hide resolved
source/patterns/@aws-solutions-constructs/aws-lambda-sqs-lambda/README.md
Outdated
Show resolved
Hide resolved
source/patterns/@aws-solutions-constructs/aws-s3-stepfunctions/README.md
Outdated
Show resolved
Hide resolved
source/patterns/@aws-solutions-constructs/aws-events-rule-sqs/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 |
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.
One very minor change, then I think it is done!
@@ -37,12 +37,12 @@ new LambdaToSqsToLambda(this, 'LambdaToSqsToLambdaPattern', { | |||
producerLambdaFunctionProps: { | |||
runtime: lambda.Runtime.NODEJS_14_X, | |||
handler: 'index.handler', | |||
code: lambda.Code.fromAsset(`lambda/producer-function`) | |||
code: lambda.Code.fromAsset(`producer-function`) |
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.
producer-lambda and consumer-lambda to match Python and Java examples
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.
I realize this follows the terms in my original comment - sorry about that...
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 #552 , if available:
Description of changes:
-Added Python minimal deployment
-Removed Initalizers
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
fixes #552