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 bunyan logging #5

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

Add bunyan logging #5

wants to merge 4 commits into from

Conversation

thejsj
Copy link
Member

@thejsj thejsj commented Jun 7, 2017

No description provided.

anandkumarpatel
anandkumarpatel requested changes Jun 7, 2017
View reviewed changes
Copy link
Contributor

@anandkumarpatel anandkumarpatel left a comment

Choose a reason for hiding this comment

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

Would be cool if you added ponos and made this a worker server which listens to khronos for the 5 min event so you dont have to deal with cron. just 1 extra file :)

lib/aws.js
if (DRY_RUN) {
console.log(JSON.stringify(postData))
}
log.debug({ postData }, 'DRY RUN: Data to be posted to cloudwatch')
Copy link
Contributor

Choose a reason for hiding this comment

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

dry run? dry run was removed here

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I just made these debug statements. So you just have to change the level. Will remove the DRY RUN stuff from the log message.

@thejsj
Copy link
Member Author

thejsj commented Jun 7, 2017

Would be cool if you added ponos and made this a worker server which listens to khronos for the 5 min event so you dont have to deal with cron. just 1 extra file :)

The thing I don't like about this is that now we're running a node container 100% of the time (with the associated memory implications, for just a cron job). Maybe we should stick furry-cactus in palantiri as a worker? That's where it seems it should go. Then we don't pay for the overhead of running a ponos server. I don't think the ponos server is THAT big of a deal, but I do think we could do a better job of managing this.

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

@anandkumarpatel anandkumarpatel anandkumarpatel requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@thejsj @anandkumarpatel