Add recipe for sqlserver #425
Conversation
|
Hi @olivielpeau |
|
This PR will hopefully satisfy #333 |
There was a problem hiding this comment.
Had one question, other than that this looks good to me, thanks @mlcooper!
I'll spend some time to fix the CI
templates/default/sqlserver.yaml.erb
Outdated
| - host: HOST,PORT | ||
| username: my_username | ||
| password: my_password | ||
| <% if @init_config.empty? -%> |
There was a problem hiding this comment.
Shouldn't <%= JSON.parse(({'init_config' => @init_config, 'instances' => @instances}).to_json).to_yaml %> work as expected even when @init_config is empty?
(wondering if we could get rid of the if clause completely)
There was a problem hiding this comment.
I did try that, however when I just use <%= JSON.parse(({'init_config' => @init_config, 'instances' => @instances}).to_json).to_yaml %>, when @init_config is empty, the chef spec test fails with:
1) datadog::sqlserver config 2 renders expected YAML config file
Failure/Error: expect(YAML.load(content).to_json).to be_json_eql(YAML.load(expected_yaml).to_json)
Expected equivalent JSON
Diff:
@@ -1,5 +1,6 @@
{
- "init_config": null,
+ "init_config": {
+ },
"instances": [
{
"command_timeout": 30,
# ./spec/integrations/sqlserver_spec.rb:135:in `block (4 levels) in <top (required)>'
# /home/coopem67/.chefdk/gem/ruby/2.3.0/gems/chefspec-5.4.0/lib/chefspec/matchers/render_file_matcher.rb:120:in `matches_content?'
# /home/coopem67/.chefdk/gem/ruby/2.3.0/gems/chefspec-5.4.0/lib/chefspec/matchers/render_file_matcher.rb:13:in `matches?'
# ./spec/integrations/sqlserver_spec.rb:134:in `block (3 levels) in <top (required)>'
Finished in 5.16 seconds (files took 5.76 seconds to load)
20 examples, 1 failure
Failed examples:
rspec ./spec/integrations/sqlserver_spec.rb:133 # datadog::sqlserver config 2 renders expected YAML config file
There was a problem hiding this comment.
Thanks @mlcooper for the explanation.
Since the behavior of having init_config set to the empty map ({}) instead of null is not a problem per se, what do you think of removing the if clause from the template and setting the first line of the expected yaml in the spec test to init_config: {}?
|
Hi @olivielpeau I got rid of the |
|
Thanks, looks ready to go! The CI failure was transient, a re-run fixed it. Merging |
No description provided.