Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Code review #1

Open
forrestbrazeal opened this issue Jul 19, 2020 · 0 comments
Open

Code review #1

forrestbrazeal opened this issue Jul 19, 2020 · 0 comments

Comments

@forrestbrazeal
Copy link
Collaborator

Huge congrats on completing this challenge!

Epic accomplishment, great work all around. You're obviously not new to programming, and I appreciated some of the advanced touches you added, such as including rate limit protections on your API. Great work.

Just a few overall code comments as you take your AWS and serverless development skills to the next level:

  • Consider using structured JSON objects as logs, not print statements -- this makes them easier to search and filter in log analysis platforms. (Very important if you have lots of users hitting your Lambdas!) Here is a great post with some ideas on how to structure Lambda logs: https://theburningmonk.com/2018/01/you-need-to-use-structured-logging-with-aws-lambda/
  • I notice you are updating the counter value and then making a second call to DynamoDB to retrieve your value to return. You should be able to make this work without the second call. Look into what DynamoDB can return from the update operation.
  • Suggestion: don't give your DynamoDB table a custom name. This removes the ability to update some attributes without replacement in CloudFormation. Instead, let CloudFormation name the table, and pass the generated table name to your function as an environment variable or SSM parameter.
  • Also, don't hardcode your DynamoDB table name in the Lambda code. Instead, pass it as an environment variable or SSM parameter. (This will be helpful if you ever need to deploy multiple version of your stack in the same environment).
  • Suggestion: look into "on demand" billing mode for your DynamoDB table rather than provisioned capacity -- this will likely be a cost savings for a small site like this.
  • Consider initializing your boto resources outside the Lambda handler, in global space - thus, they will only need to be initialized on cold start, which can speed up function performance.
  • Looks like your IAM role is a bit permissive -- do you really need all those DynamoDB access rights? See how far you can scope that down.
    Good luck, and I look forward to sharing your amazing work with the community.
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant