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

Make integers come out in JSON as integers. #121

Merged
merged 1 commit into from
Dec 30, 2013
Merged

Make integers come out in JSON as integers. #121

merged 1 commit into from
Dec 30, 2013

Conversation

bobtfish
Copy link
Contributor

There are places (for example if you pass keepalive/threshold values to
the keepalive check in custom_client) where it matters about the type
of the variable coming from the JSON - Sensu looks for .is_a?(Integer)
and so if you quote the integers in the client config then it breaks.

There is already a to_type method which was replicated in a number of
places - however it wasn't recursive, and (at least in the case of
the client config) you can have a hash of hashes.

I've pulled this method out into a common mixin, and reused it from all
the places which use it currently + added it to the custom= methods
in providers so that things are munged on the way in, and also to the
client_config type to make data comparisions work as expected

There are places (for example if you pass keepalive/threshold values to
the keepalive check in custom_client) where it matters about the type
of the variable coming from the JSON - Sensu looks for .is_a?(Integer)
and so if you quote the integers in the client config then it breaks.

There is already a to_type method which was replicated in a number of
places - however it wasn't recursive, and (at least in the case of
the client config) you can have a hash of hashes.

I've pulled this method out into a common mixin, and reused it from all
the places which use it currently + added it to the custom= methods
in providers so that things are munged on the way in, and also to the
client_config type to make data comparisions work as expected
jamtur01 added a commit that referenced this pull request Dec 30, 2013
Make integers come out in JSON as integers.
@jamtur01 jamtur01 merged commit 2f79d3b into sensu:master Dec 30, 2013
@jamtur01
Copy link
Contributor

Seems reasonable and a lot more DRY. Merging.

Thanks! 👍

@jlambert121
Copy link
Contributor

@bobtfish it looks like this breaks any module using the sensu defines on anything other than ruby 1.8.7 with this error:

     Puppet::Error:
       Could not autoload puppet/type/sensu_check: cannot load such file -- puppet_x/sensu/to_type on node justins-macbook-pro.local

Is this something you've run into?

@bobtfish
Copy link
Contributor Author

This is not something I've seen - I'm using ruby 1.9.3 but masterless - I assume you're running with a master - do you see to_type.rb getting pluginsync'd?

@jlambert121
Copy link
Contributor

It was travis I first noticed it on, but running spec tests locally on my mac (puppet 3.4.0, ruby 2.0.0, rspec-puppet 1.0.1) shows it as well. I'm guessing it has something to do with the way puppet/rspec-puppet autoloads stuff.

@bobtfish
Copy link
Contributor Author

I can't see it on travis currently, can you point it out?

Works for me on my real (masterless) puppet (ruby 1.9.3p0, puppet 3.4.0), and on my laptop running the tests (puppet 3.4.0, rspec-puppet 1.0.1, ruby 1.9.3p448 and/or 2.0.0-p247) - sorry!

@jlambert121
Copy link
Contributor

Ahh, sorry - not this module itself I'm seeing the issue, but other modules that call sensu::check. For example, if you uncomment sensu from the fixtures on this module you can duplicate the error. https://github.com/evenup/evenup-riakdev

I like this solution, just need to figure out how to make sure things using this module don't break unnecessarily.

@bobtfish
Copy link
Contributor Author

Interesting - as my home puppet rig calls sensu::check from other modules (e.g. profile::sensu_client pulls in a load of default checks)...

I'll try to dig into this tomorrow!

@jlambert121
Copy link
Contributor

Thanks! I spent some time on it yesterday and my type/provider/rspec-puppet skills just aren't enough to find this. Here's a travis build that shows it - 1.8.7 works fine, 1.9.3, 2.0.0 complains. https://travis-ci.org/evenup/evenup-apache/builds/17071647

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

Successfully merging this pull request may close these issues.

3 participants