Skip to content
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.

Sign up for GitHub

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

Jump to bottom

add options #5

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

add options #5

wants to merge 9 commits into from

Conversation

kentokento
Copy link 

Usage:
    dynamodb-create-cloudwatch-alarms (-s <sns_topic_arn>) [-r <region>] [-p <prefix>] [--debug] 
    dynamodb-create-cloudwatch-alarms [-h | --help]

Options:
     -s <sns_topic_arn>    For sending alarm ( require )
     -r <region>           AWS region
     -p <prefix>           DynamoDB name prefix
     --debug               Don't send data to AWS

@lra
Copy link
Member

lra commented Feb 3, 2016

Thanks, we'll review it as soon as the tests are green, can you check the error on CircleCI?

@kentokento
Copy link
Author

Thank you very much for the information. I will check it.

@kentokento
Copy link
Author

fixed

lra
lra reviewed Feb 4, 2016
View reviewed changes
.gitignore
@@ -1 +1,2 @@
*.pyc
dynamodb_create_cloudwatch_alarms.egg-info
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

prefix with /?

@lra
Copy link
Member

lra commented Feb 4, 2016

Thanks for your contribution!
I made some comments, if you could address them, it would be great.

lra
lra requested changes Nov 8, 2016
View reviewed changes
.gitignore
@@ -1,2 +1,3 @@
*.pyc
dynamodb_create_cloudwatch_alarms.egg-info
exec.sh
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like your own stuff, not needed in here.

dynamodb_create_cloudwatch_alarms/main.py
def __count():
i[0] += 1
if i[0] > 10:
time.sleep(5)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why sleeping?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lra lra lra requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@kentokento @lra