Fix #19792 - cryptic error when missing deep_merge gem #124
Conversation
| rescue LoadError | ||
| Hiera.warn "Ignoring configured merge_behavior" | ||
| Hiera.warn "Must have 'deep_merge' gem installed." | ||
| @config[:merge_behavior] = :native |
There was a problem hiding this comment.
Falling back to :native for the merge behavior is the right behavior, but I think this is introducing test failures (https://travis-ci.org/puppetlabs/hiera/jobs/5823971#L215). This implementation means that there's no way to leave deep merging on for the purposes of testing. This means means that unit tests around deep merging either have to be disabled if the deep_merge gem isn't installed, we have to mock out require (which is incredibly dangerous) or this needs to be extracted out. If this was extracted into a method then this could simply be stubbed out for the purposes of tests.
|
d6cf021 solves this problem. This also allows us to stub out any other config validation logic that may have side-effects. |
The configuration loader will try to pre-emptively load 'deep_merge' if the 'deep' or 'deeper' options were given to :merge_behavior. If the load fails, it will warn the user and revert back to 'native'.
|
I've cleaned up the branch history since it really only deserves one commit - they are all related changes. |
|
@justenwalker looks great! I'm merging this into master, but I want to note that as of 2013-03-27 we have a 1.x branch. It's identical to master so we might want to cut the 1.2 release from master, but that's release eng's job to make the call. |
Fix #19792 - cryptic error when missing deep_merge gem
|
Merged into master as 995a021. This should be released in 1.2.0. Thanks again for the contribution! -Adrien |
The configuration loader will try to pre-emptively load 'deep_merge' if
the 'deep' or 'deeper' options were given to :merge_behavior.
If the load fails, it will warn the user and revert back to 'native'.