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

Add agent host info for Rackspace Cloud Monitoring #2442

Merged
merged 10 commits into from Jan 6, 2014
Merged

Add agent host info for Rackspace Cloud Monitoring #2442

merged 10 commits into from Jan 6, 2014

Conversation

kfafel
Copy link

@kfafel kfafel commented Nov 29, 2013

Implements these Rackspace Cloud Monitoring API calls - http://docs.rackspace.com/cm/api/v1.0/cm-devguide/content/service-agent-host_info.html

@coveralls
Copy link

Coverage Status

Coverage decreased (-1.18%) when pulling e95b780 on kfafel:AddAgentHostInfo into 89b5edb on fog:master.

@geemus
Copy link
Member

geemus commented Nov 30, 2013

@krames - could you review, thanks!

class Mock
def get_cpus_info(agent_id)

agent_id = Fog::Rackspace::MockData.uuid
Copy link
Member

Choose a reason for hiding this comment

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

Awesome job on the mocks!

@krames
Copy link
Member

krames commented Dec 6, 2013

Sorry about the delay in reviewing your PR. (It's been a crazy week).

The code looks good to me, but could I get you to write some tests to ensure that a future committer doesn't break anything? You can find a good example of tests here -> https://github.com/fog/fog/blob/master/tests/rackspace/requests/queues/claim_tests.rb

@krames
Copy link
Member

krames commented Dec 11, 2013

@kfafel Here is a gist with our sample -> https://gist.github.com/krames/7919623

Let me know if you need any more help!

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling cba844e on kfafel:AddAgentHostInfo into c5e6e2a on fog:master.

@krames
Copy link
Member

krames commented Dec 16, 2013

@kfafel It looks like one of the travis tests had a random failure. I restarted it and everything looks good. Let me know if you need any more help with tests.

@kfafel
Copy link
Author

kfafel commented Dec 16, 2013

@krames great to hear on the Travis tests! On the Shindo tests, I worked on them some this weekend... Instead of simply testing for a Response Code of 200, I'm trying to go a little deeper, and test for the right data format being returned. If it turns into too much hassle, I'll revert back to the Response Codes just to get something working out there.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 4934ae7 on kfafel:AddAgentHostInfo into 2a4ac7f on fog:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 90bbc02 on kfafel:AddAgentHostInfo into 2a4ac7f on fog:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-9.57%) when pulling 976a1c8 on kfafel:AddAgentHostInfo into 2a4ac7f on fog:master.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 43b3258 on kfafel:AddAgentHostInfo into 1044b3b on fog:master.

@kfafel
Copy link
Author

kfafel commented Dec 23, 2013

@krames -- okay, I think I've got the shindo tests working well. The tests are looking for more than a response status code, but not too much more -- basically just the body to be a hash. I was going to look for specific values, but the output was slightly different for a couple agent calls. The lowest common denominator was an hash for a body. I could go deeper, but I was trying to balance between simplicity of code vs going overboard with little return. Thoughts?

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 43b3258 on kfafel:AddAgentHostInfo into 1044b3b on fog:master.

@krames
Copy link
Member

krames commented Jan 6, 2014

@kfafel more information is always preferable, but this at least exercises the call and that's the most important thing.

It looks like this commit is missing some files. When I execute it locally, it is complaining about not finding the list_agents request. Can you verify that you have pushed all your changes up to the branch?

 Fog::Rackspace::Monitoring | agent token (rackspace, rackspace_monitoring) /Users/kyle.rames/Projects/fog/lib/fog/core/service.rb:102:in `require': cannot load such file -- fog/rackspace/requests/monitoring/list_agents (LoadError)
    from /Users/kyle.rames/Projects/fog/lib/fog/core/service.rb:102:in `block in setup_requirements'
    from /Users/kyle.rames/Projects/fog/lib/fog/core/service.rb:101:in `each'
    from /Users/kyle.rames/Projects/fog/lib/fog/core/service.rb:101:in `setup_requirements'
    from /Users/kyle.rames/Projects/fog/lib/fog/core/service.rb:60:in `new'
    from tests/rackspace/models/monitoring/agent_token_tests.rb:2:in `block in <top (required)>'
    from /Users/kyle.rames/.rvm/gems/ruby-1.9.3-p392@fog_dev/gems/shindo-0.3.8/lib/shindo.rb:79:in `instance_eval'
    from /Users/kyle.rames/.rvm/gems/ruby-1.9.3-p392@fog_dev/gems/shindo-0.3.8/lib/shindo.rb:79:in `tests'
    from /Users/kyle.rames/.rvm/gems/ruby-1.9.3-p392@fog_dev/gems/shindo-0.3.8/lib/shindo.rb:38:in `initialize'
    from /Users/kyle.rames/.rvm/gems/ruby-1.9.3-p392@fog_dev/gems/shindo-0.3.8/lib/shindo.rb:13:in `new'
    from /Users/kyle.rames/.rvm/gems/ruby-1.9.3-p392@fog_dev/gems/shindo-0.3.8/lib/shindo.rb:13:in `tests'
    from tests/rackspace/models/monitoring/agent_token_tests.rb:1:in `<top (required)>'
    from /Users/kyle.rames/.rvm/gems/ruby-1.9.3-p392@fog_dev/gems/shindo-0.3.8/lib/shindo/bin.rb:61:in `load'
    from /Users/kyle.rames/.rvm/gems/ruby-1.9.3-p392@fog_dev/gems/shindo-0.3.8/lib/shindo/bin.rb:61:in `block (2 levels) in run_in_thread'
    from /Users/kyle.rames/.rvm/gems/ruby-1.9.3-p392@fog_dev/gems/shindo-0.3.8/lib/shindo/bin.rb:58:in `each'
    from /Users/kyle.rames/.rvm/gems/ruby-1.9.3-p392@fog_dev/gems/shindo-0.3.8/lib/shindo/bin.rb:58:in `block in run_in_thread'

Good work BTW!!! 👍

@kfafel
Copy link
Author

kfafel commented Jan 6, 2014

@krames ... oops, you're right. I missed adding a couple files. try now.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.49%) when pulling 881813a on kfafel:AddAgentHostInfo into 1044b3b on fog:master.

@krames
Copy link
Member

krames commented Jan 6, 2014

@kfafel Looks good to me. Are you ready for me to merge it?

@kfafel
Copy link
Author

kfafel commented Jan 6, 2014

@krames go for it! :)

krames pushed a commit that referenced this pull request Jan 6, 2014
Add agent host info for Rackspace Cloud Monitoring
@krames krames merged commit 1a38adc into fog:master Jan 6, 2014
@krames
Copy link
Member

krames commented Jan 6, 2014

Thanks! 👍

@kfafel kfafel deleted the AddAgentHostInfo branch January 6, 2014 22:06
@geemus
Copy link
Member

geemus commented Jan 7, 2014

Thanks!

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.

None yet

4 participants