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.

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

Initial commit of Pingdom Agent. Grabs Pingdom checks to pass along to other agents. #369

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

Conversation

viable-hartman
Copy link

I use the memory hash to store the last Pingdom check and only create events if the status has changed from the previous check in. I've been running this internally for the last couple days without any issue, and its been updating agents it sources perfectly.

I've provided real data from my Pingdom queries in the rspec, so all should be well.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) when pulling ffeed80 on viable-hartman:Pingdom_Agent into 577b95f on cantino:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) when pulling 7f186ad on viable-hartman:Pingdom_Agent into 577b95f on cantino:master.

@0xdevalias
Copy link
Member

@viable-hartman Sounds cool!
@cantino / @dsander It seems across the agents there are a number of different web access methods (eg rest-client) in use. Do you think there's a nice way to abstract this? Ideally in a pluggable way?

@viable-hartman
Copy link
Author

There is always a way; I'll help look into it in my spare time.

@cantino
Copy link
Member

cantino commented Jun 4, 2014

@alias1 @viable-hartman faraday is probably the right way to abstract it, as it takes many backends and we already require it.

@@ -64,6 +64,9 @@ gem 'wunderground', '~> 1.2.0'
gem 'forecast_io', '~> 2.0.0'
gem 'rturk', '~> 2.12.1'

# Required for the pingdom agent
gem 'rest-client', '~> 1.6.7', require: false
Copy link
Member

Choose a reason for hiding this comment

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

If you can, let's use faraday or typhoeus instead and not add a new dependency.

@cantino
Copy link
Member

cantino commented Jun 4, 2014

Thanks @viable-hartman! This is a cool idea for an Agent, and I like your specs!

Couple things:

  • We've recently added a credential Liquid tag in Added a liquid tag to access user credentials #348. Perhaps pingdom_apikey could be set to {% credential pingdom_api_key %} by default, so users know that they could make that pingdom_api_key Credential, or they could paste in their api key directly if they want to. (/cc @dsander what do you think?) You'll need to make sure you're Liquid parsing that value then.
  • (minor) indention should be 2 spaces

@dsander
Copy link
Collaborator

dsander commented Jun 4, 2014

I think its a good idea to use the liquid filter.

@viable-hartman If you want to go with it you need to include LiquidInterpolatable to you agent
include LiquidInterpolatable and use the interpolate_string method like this interpolate_string(options['pingdom_apikey'], {})

@cantino
Copy link
Member

cantino commented Jun 29, 2014

If you merge in master, you should be able to just use interpolated now instead of option to get Liquid interpolation for free.

@cantino
Copy link
Member

cantino commented Jul 29, 2014

Hey @viable-hartman, are you still planning to work on this? If not, I might try to get it updated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants