[PUP-8144] Merge Strategy 'first|unique|hash' documentation is unclear Created: 2017/11/09  Updated: 2017/12/13  Resolved: 2017/12/06

Status: Closed
Project: Puppet
Component/s: Docs
Affects Version/s: None
Fix Version/s: PUP 4.10.10, PUP 5.3.4, PUP 5.4.0

Type: Improvement Priority: Normal
Reporter: Brian Vanderbusch Assignee: Claire Cadman
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Relates
relates to PUP-8146 consider combined merge strategies fo... Open
Template:
Acceptance Criteria:
  • add documentation on purpose or usage situation for this merge strategy
  • Include details of how the various strategies are selected by hiera
  • Indicate if the `|` vertical pipe indicates that this is a configurable strategy (could I say use `strategy => 'unique|hash` instead?)
Team: Platform Core
Release Notes: Not Needed
QA Risk Assessment: Needs Assessment

 Description   

The documentation for the [`first|unique|hash`](https://puppet.com/docs/puppet/5.3/hiera_merging.html#strategy--firstuniquehash) merge strategy is brief, and does not explain usage or functionality of the merge strategy in a similar way to the documentation for the other merge strategies.

The entirety of the current documentation for this strategy is one sentance:

> Same as the string versions of these merge behaviors.

This doesn't tell me anything, since obviously it's not the same, or it wouldn't be an additonal option. Mainly, I believe it's implying that hiera will try to select the appropriate strategy based on the data-type. However that raises the question of how the `unique` or `hash` strategy would be applied to a key when `first` is also present (which ignores all merge's entirely).



 Comments   
Comment by Brian Vanderbusch [ 2017/11/09 ]

So an example use case that I want to determine if this merge strategy will work:

My default merge strategies for most modules in my repo is `first`, however, I want to use either `unique` or `hash` (depending on the parameter type) for all of the parameters for an individual module that contains layer 3 data sources.

The module has dozens of parameters, and I'd prefer my `lookup_strategy` config in `common.yaml` be simple:

lookup_strategy:
'^some_profile:.*)':
strategy: 'unique|hash'

As opposed to listing out the dozens of keys just to specify the same merge behavior for each hash or array parameter in the module.

The documentation for `first|unique|hash` leaves me to guess that it tries to combine `unique` and `hash` by evaluating the type of data provided... but makes no sense that `first` would be there as well in this scenario.

Comment by Henrik Lindberg [ 2017/11/10 ]

There is no such thing as "one or the other strategy determined by the data type" the documentation is just showing you the available individual options using | to mean 'or' between them. You cannot combine multiple strategies into one.

You are right in that it would make sense to be able to specify different strategies for different data types - but that is rather difficult since that places the buggy before the horse so to say - the search needs to know what to search for and how to combine the result when it starts, it would otherwise have to try the strategies in sequence which would be slow.

What we could do is to improve so the strategy is selected based on the data type being looked up (i.e. when given explicitly to lookup), or when a parameter is typed (the APL case). But if there is no information - for example class foo($x) there is no such type information to go on. I guess that could be detected and an error raised in that scenario (either break out that key with a separate lookup_option, or add a data type to the parameter).

Ping Thomas Hallgren - any thoughts on the possibility to delay the selection of the strategy until finding a piece of data that is either an Array or Hash?

This ticket is now a documentation ticket, and we should naturally make the documentation clearer (I find the documentation to at least be correct, albeit somewhat terse and thus easy to misunderstand). After som discussion on the merit of a possible additional feature we can create a separate ticket for that.

I think we can fix the documentation by simply not using | to mean 'or' since readers may not be familiar with that notation.
We need to fix that in the documentation of the lookup function itself as well. I am therefore moving this ticket to the PUP project and will tag it with a DOCS component for the update of the manually written docs page in question.

Comment by Thomas Hallgren [ 2017/11/10 ]

Not sure what good that would do? If you don't specify a merge strategy, then `first` will be the default and no merge will take place.

Comment by Henrik Lindberg [ 2017/11/10 ]

I created PUP-8146 for the possible addition of combined strategies (discussion to continue there).

Comment by Kari Halsted [ 2017/12/06 ]

Ah, yeah, I see what's going on in the docs. The behavior of

{strategy=>first}, {strategy=>hash}, {strategy=>unique}, and {strategy=>deep...} is already described in the sections higher up in the page, and we should just remove this section which is causing confusion (and also the one below it about deep).

The phrase "Same as the string versions of these merge behaviors" is trying (and failing) to say that saying "first" is the same as saying {strategy=>first}

.

Claire, can you take out the two sections "

{'strategy' => 'first|unique|hash'}

" and "

{'strategy' => 'deep', <DEEP OPTION> => <VALUE>, ...}

"?

Comment by Claire Cadman [ 2017/12/06 ]

Content rearranged for clarity.

Comment by Brian Vanderbusch [ 2017/12/07 ]

Bingo.

Generated at Thu Feb 27 00:48:18 PST 2020 using JIRA 7.7.1#77002-sha1:e75ca93d5574d9409c0630b81c894d9065296414.