Send mapped statuses #9
Conversation
|
Hm... :/ Not sure why the test failed, the new snapshot looks OK to me. |
| "proxy" => proxy, | ||
| "body" => @json_body ? body.to_json : body, | ||
| "keywords" => keywords, | ||
| }, | ||
| }.merge(statuses), |
There was a problem hiding this comment.
Why are you merging instead of just building the hash?
There was a problem hiding this comment.
Well, from my perspective it looks cleaner, but if you prefer explicit style, I can refactor :)
There was a problem hiding this comment.
I am good, I just thought there is something behind this.
|
|
||
| def process_statuses | ||
| stats = @body["metrics"]["resources"]["values"].collect { |s| [s[0], s[2]] } | ||
| .to_h |
There was a problem hiding this comment.
This should be resistant to missing keys, please add rescue method block and log a warning and return stats with all values set to zero.
You do not need to break at 80th column, unless you prefer to do it.
| applied: stats["changed"] + stats["corrective_change"], | ||
| failed: stats["failed"] + stats["failed_to_restart"], | ||
| pending: stats["scheduled"], | ||
| other: stats["restarted"] + stats["skipped"] + stats["out_of_sync"], |
There was a problem hiding this comment.
The problem why tests fail is that you use Ruby symbols here, for the best performance I am using strings everywhere since JSON deserialization is into strings by default.
| pending: @body["status"]["pending"] || 0, # It's only present in check mode | ||
| other: @body["status"]["skipped"], | ||
| } | ||
| end |
There was a problem hiding this comment.
I suggest to add a rescue block and return zeroes in case anything goes wrong, drop a warning into logs as well. These keys should be always present but for consistency with Puppet.
|
I have just realized that not all output keys are strings actually, I am fixing that in another PR. Adding a test to ensure this so we won't run into this in the future. |
|
I pushed a huge refactoring of tests, there are now tests for both Puppet and Ansible and for all fixtures we have (tests are created dynamically). Also there is an extra test that fails when there is a symbol hash key instead of string. I moved all fixtures from |
|
Please rebase, simply delete all snapshots and run tests once to write them over: |
|
@lzap, updated. |
|
It works fine, thanks. |
No description provided.