(wip) some patches to get things working with puppet-unbound #1
Conversation
It is possible to explode the Hiera overrides and generate a table in the reference.
b4ldr
force-pushed
the
add-hiera-overrides
branch
from
61d404a
to
7341625
Compare
|
I made som progress on parsing the common.yaml file or should i say files wit no interpolation tokens see here |
b4ldr
force-pushed
the
add-hiera-overrides
branch
from
7341625
to
fe0615d
Compare
b4ldr
force-pushed
the
add-hiera-overrides
branch
from
fe0615d
to
102a588
Compare
| def load_config | ||
| return unless File.exist?(config_path) | ||
|
|
||
| config = YAML.load(File.read(config_path)) | ||
|
|
||
| unless config['version'] == 5 | ||
| raise "Unsupported version '#{config['version']}'" | ||
| log.warn("Unsupported version '#{config['version']}'") | ||
| return |
There was a problem hiding this comment.
i think this achieves "Graceful fallback if legacy Hiera data is used" although see the todo above, currently we load the config for every instance
There was a problem hiding this comment.
Interesting idea to override the default. I didn't really think of that. I do think we should do that at another level then. This is just the markdown implementation, but that means HTML and JSON don't benefit from it. I initially wrote code mostly just so I could demonstrate it could be done in the reference, but it's really not the right thing.
| regex.match(entry) do |match| | ||
| full_path = File.join(datadir, entry) | ||
| interpolations = {} | ||
| regex.match(entry) do |match| |
There was a problem hiding this comment.
I had the chdir there so the globbed paths were relative to datadir rather than absolute. This reduced the risk of any oddities with the regex here caused by the unused part.
There was a problem hiding this comment.
hmm im not sure thats the best idea. 1) im not convinced there is a problem to solve (do you have some examples, even contrived ones) 2) if there is a problem at best we are just reducing the risk of triggering it by using chdir
There was a problem hiding this comment.
The alternative is to normalize it again. For example (using pathname):
data_dir = Pathname.new(datadir)
data_dir.glob('**').each do |entry|
next unless x.file?
regex.match(entry.relative_path_from(data_dir)) do |match|
# ...
end
endNote that I also chose to glob everything so that we don't have to think about extensions (yaml vs yml). The regex matching will take care of that.
| @@ -82,7 +82,7 @@ Options: | |||
| Default value: `<%= value_string(defaults[param[:name]]) %>` | |||
|
|
|||
| <% end -%> | |||
| <% if hiera_overrides[param[:name]] -%> | |||
| <% if !hiera_overrides.empty? && hiera_overrides[param[:name]] -%> | |||
There was a problem hiding this comment.
I didn't have the empty check because in Ruby a missing key in a hash returns nil so it wouldn't be triggered. I think this change is redundant.
There was a problem hiding this comment.
hiera_overrides by default returns [] and defined as @return [Array[Tuple[String, Hash[String, String], Any]]] not sure if this is by design or not but i got a stack trace when it was empty: unable to cast string to int (or something like that)
b4ldr
force-pushed
the
add-hiera-overrides
branch
from
102a588
to
f1a1bb8
Compare
|
responded to most things inline, in general im pretty sure i agree with you. however from a practical point of view. im not familiar with yard or puppet-strings, and even ruby its self is one of my weaker languages. So when it comes to moving things to "the correct place" i definitely agree but at the same time. I took a quick look and couldn't work out where the correct place would be. If you have pointers that would be great if not i could maybe ping the initial thread? |
ekohl
force-pushed
the
add-hiera-overrides
branch
from
2340d1b
to
44a8de6
Compare
ill probably continue working on this but just in case it slips my mind i thought id send the fixes i already needed to apply