Skip to content

chef-handler-datadog, v0.3.0.dev#40

Merged
miketheman merged 33 commits intomasterfrom
0.3.0
Jan 23, 2014
Merged

chef-handler-datadog, v0.3.0.dev#40
miketheman merged 33 commits intomasterfrom
0.3.0

Conversation

@miketheman
Copy link
Copy Markdown
Contributor

This is a bit of a largish changeset, mostly due to adding testing into the mix to enable catching any regressions when refactoring.

Some review would be appreciated. I tried to set out the high-level details of the changes in CHANGELOG.md.

Comments are welcome. Target date for release is Wednesday, January 22 - at which time this branch will be merged, the version promoted to 0.3.0, and released to rubygems.

Chef 0.9.x has been end-of-life and should be replaced by Chef 10.
The method was backported from 10.x when 0.9 was still in cirtuclation,
and is no longer needed.
- Drop the instance variables for api key and app key, as were used
  in only one place
- Rename `opts` to `config`, and cast and instance variable, similar
  to other handlers, helps with testing
Removes some of the decision-making from the init method, moves
decision to a private method and simplifies the main report method.
Simplify the `report` method, reduce cyclomatic complexity by 1
The new method also uses implict `begin` statements, since the method
does nothing else.
- Moves more work out of `report`
- Reduces Cyclomatic Complexity by 2
- Improves readability
If the application_key isn't provided, then warn the user before
calling the tags endpoint.
Fixes #39

We were using the `evt` object which was reporting the result of a
passed/failed event instead of the `rc` object that contains the
response for the tag-setting method.
- set up dependencies with Appraisals
- test more specific versions
- replace tailor with rubocop. config and test will follow
The Chef + JSON gem relationship is rocky and fraught with peril.
Specifiying the highest avaialble json version for these releases
should allow for testing them in isolation.
It does nothing, and will be replaced with RSpec testing.
Add control file with the rules (cops) that we have purposely
overridden with this project's current values.
Using VCR with pre-recorded cassettes, the test suite won't actually
interact with the Datadog API unless new tests are added.
This has the added value of increasing speed of the test suite, as
well as not relying on the performance of the API, or cluttering up
a test account.
Fixes #31

Since a dogapi client will fail when trying to submit tags without
the correct credentials, prevent the attempt by failing the Handler
run at that point.

Also moves the `before` section of the configuration to an `each`
section in the test suite, to better handle individual suites
setting their own config.
Fixes #37

If a formatted resources list exceeds 4000 characters, we lose the
exception and backtrace from the event.
It seems likkely that the backtrace has more value than the resources
list at this point, swapping the order seems least harmful.
The removal of `defined_at` Class method in 81c53e0 broke the
event reporting when any resources are updated, since the test cases
weren't covering that evenatuality yet.
Adding actual resource definitions and updates triggers the problem.

Calling `defined_at` on the Resource itself provides the output we want.

Also removed some extra require lines, since these are being pulled in
from a longer tree of requires that extends beyond a Handler's
responsibility, but are made available here.
.rubocop.yml Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

nitpick: there are a couple typos here

@conorbranagan
Copy link
Copy Markdown

Overall this looks really great. Well done!

miketheman added a commit that referenced this pull request Jan 23, 2014
chef-handler-datadog, v0.3.0.dev
@miketheman miketheman merged commit c88e470 into master Jan 23, 2014
@miketheman miketheman deleted the 0.3.0 branch April 28, 2014 15:40
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.

2 participants