-
Notifications
You must be signed in to change notification settings - Fork 236
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
Add option to stop the database process #223
base: v1
Are you sure you want to change the base?
Conversation
index.js
Outdated
@@ -88,7 +88,10 @@ class ServerlessDynamodbLocal { | |||
convertEmptyValues: { | |||
shortcut: "e", | |||
usage: "Set to true if you would like the document client to convert empty values (0-length strings, binary buffers, and sets) to be converted to NULL types when persisting to DynamoDB.", | |||
} | |||
}, | |||
userStop: { |
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.
Shall we make the userStop more meaningful? e.g; stopDbAfterSeeding or feel free to propose a better name.
README.md
Outdated
@@ -57,6 +57,7 @@ All CLI options are optional: | |||
--migrate -m After starting DynamoDB local, create DynamoDB tables from the Serverless configuration. | |||
--seed -s After starting and migrating dynamodb local, injects seed data into your tables. The --seed option determines which data categories to onload. | |||
--convertEmptyValues -e Set to true if you would like the document client to convert empty values (0-length strings, binary buffers, and sets) to be converted to NULL types when persisting to DynamoDB. | |||
--userStop After starting dynamodb local and all migrations and seeding is completed (if set to run), process will stay open until user terminates running process. When terminate signal received, it will stop the database on the running port. |
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.
Here, will you be able to make the formatting of the option, consistent with the rest, e.g; having shortcut before the statement, spacing?
index.js
Outdated
@@ -258,6 +271,24 @@ class ServerlessDynamodbLocal { | |||
}).filter((n) => n); | |||
} | |||
|
|||
_listenForTermination() { |
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.
As I understood, you have used underscore before the name of the function to differentiate it a private? But when I look at the code, so far this practice hasn't been followed. So shall we keep it without the underscore for the moment to keep the consistency?
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.
Hi @dwbelliston . Thanks a lot for the PR. I have added a few comments. Will you be able to resolve them so that I could merge it?
okay check those changes are what you anticipated. thanks for the review! |
Adds options for user to provide flag to keep process running until user send terminate signal.
This allows for the serverless-dynamodb-local plugin to call the dynamodb-local 'stop' command and still have the db instance in memory and the associated port it should kill the database process on.