-
Notifications
You must be signed in to change notification settings - Fork 3
KMS-565: Add LocalStack and Offline Lambda Support for Local Development #74
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
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## KMS-570 #74 +/- ##
==========================================
Coverage ? 99.57%
==========================================
Files ? 139
Lines ? 2333
Branches ? 571
==========================================
Hits ? 2323
Misses ? 9
Partials ? 1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
README.md
Outdated
| To run cdk synth: | ||
| ``` | ||
| CMR_BASE_URL=https://cmr.earthdata.nasa.gov RDF4J_SERVICE_URL=http://localhost:8080 npm run offline | ||
| npm run run-synth |
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.
is this needed if you are doing it below: cdk synth --context useLocalstack=true --output ./cdk.out > /dev/null 2>&1
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.
right, that is not needed because included in 'npm run start-local'. Is there a use case for just run 'cdk synth'?
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.
Done, removed that instruction.
bin/start-local.sh
Outdated
| npm run rdf4j:start | ||
| # Wait for rdf4j server to start | ||
| sleep 10 | ||
| npm run rdf4j:setup |
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 don't think starting the db should be part of start-local. You might want to start-local multiple times and keep the data in the rdfdb.
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.
Done, removed steps to startup and setup rdf4j.
| import * as iam from 'aws-cdk-lib/aws-iam' | ||
| import * as lambda from 'aws-cdk-lib/aws-lambda' | ||
| import { NodejsFunction } from 'aws-cdk-lib/aws-lambda-nodejs' | ||
| import * as nodejsLambda from 'aws-cdk-lib/aws-lambda-nodejs' |
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 think you can also do:
import { NodejsFunction, NodejsFunctionProps } from 'aws-cdk-lib/aws-lambda-nodejs'
which I think it preferred way of doing it.
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.
Fixed
| }) | ||
| environment: this.props.environment, | ||
| // Conditionally add VPC configuration | ||
| ...(this.props.useLocalstack ? {} : { |
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.
destructure useLocalstack from props
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.
Done.
| scope: Construct, | ||
| authorizerLambda: lambda.Function | ||
| ): apigateway.IAuthorizer { | ||
| if (this.props.useLocalstack) { |
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.
destructure
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.
Done
| account: process.env.CDK_DEFAULT_ACCOUNT, | ||
| region: process.env.CDK_DEFAULT_REGION | ||
| } | ||
| const useLocalstack = app.node.tryGetContext('useLocalstack') === 'true' |
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.
const useLocalstack = app.node.tryGetContext('useLocalstack')
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.
That would not work in case user wanted useLocalstack to be false. So I fix this way and checked in:
keep 'const useLocalstack = app.node.tryGetContext('useLocalstack') === 'true' '
but in the start-local.sh change to 'cdk synth --context useLocalstack='true' --output ./cdk.out > /dev/null 2>&1'
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.
it was 'cdk synth --context useLocalstack=true --output ./cdk.out > /dev/null 2>&1'
|
So we'll want |
Done. |
Overview
What is the feature?
Add LocalStack and Offline Lambda Support for Local Development
What is the Solution?
Add LocalStack and Offline Lambda Support for Local Development
What areas of the application does this impact?
Application run in local mode.
Testing
Attachments
N/A
Checklist