Simplify postgres.yaml template #380
Conversation
|
@theckman Care to try this branch? |
Converts the postgres template to using the "simpler" rendering solution, instead of manually unpacking positional values in YAML. Using in-template replacement logic, we can modify the values in-place at the time of rendering. This does increase memory overhead slightly, as the `instances` object is copied once, as it starts out as a `Chef::ImmutableArray` that prevents modification. - Adds spec tests for specific uses brought up in #379. - Updates in-recipe documentation to show that one can use either `server` or `host` values - Removes a breaking change TODO Resolves #379 Signed-off-by: Mike Fiedler <miketheman@gmail.com>
miketheman
force-pushed
the
miketheman/postgres-template
branch
from
69d2536 to
9dc8263
Compare
|
@miketheman I'll give it a go this afternoon. If you're curious, here is the branch I have WIP that seemed to have fixed my issues (minus the breaking changes I discussed in #379): https://github.com/theckman/chef-datadog/tree/fix_postgres_yaml -- I was going to get around to cleaning it up today, but maybe now I can be lazy. |
| # empty by design | ||
| # Generated by Chef, local modifications will be overwritten | ||
|
|
||
| <%# TODO: Breaking change, remove defaults -%> |
There was a problem hiding this comment.
It's a bummer the datadog_monitor LWRP doesn't provide a mechanism for passing the raw file content in to the LWRP. Over time I've tried to avoid doing logic like this in the template file, because they can get pretty intense pretty quick.
I wonder if there is any interest in supporting that within the LWRP itself.
| instance['port'] = 5432 if instance['port'].nil? | ||
| end | ||
| -%> | ||
| <%= JSON.parse(({'instances' => instances}).to_json).to_yaml %> |
There was a problem hiding this comment.
Nice hack for avoiding there being a second document (---) in this file. Hadn't even thought about putting that before the init_config line and was instead just building a Hash in the template file and rendering that to YAML.
|
Actually, there may be one breaking change in here around the Because each item in the tables = %w(a b c)
relations = tables.map { |t| "relation_name: #{t}" }This is the only way I could get it to render like so: relations:
- relation_name: a
- relation_name: b
- relation_name: cWith this change the file rendered like: relations:
- 'relation_name: a'
- 'relation_name: b'
- 'relation_name: c' |
|
@theckman is there an issue with backing out your wrapper recipe changes? I don't think the string vs hash was ever clearly distinguished and as you've mentioned, you had to be creative in working around it, so to me that points at a bug in the approach. |
|
@miketheman I'm all 👍 for introducing this change and fixing things on my side. I had some concerns that others may have done something similar, and wanted to call it out as a consideration. I've only done it in one place so far since I'm just starting to roll out Datadog, so it's easy for me. |
There was a problem hiding this comment.
Thanks @miketheman and @theckman, changes look good! 👌
Merging, I'll release 2.7.0 - including this - tomorrow
Converts the postgres template to using the "simpler" rendering
solution, instead of manually unpacking positional values in YAML.
Using in-template replacement logic, we can modify the values in-place
at the time of rendering.
This does increase memory overhead slightly, as the
instancesobjectis copied once, as it starts out as a
Chef::ImmutableArraythat preventsmodification.
serverorhostvaluesResolves #379
Signed-off-by: Mike Fiedler miketheman@gmail.com