Le jeudi 01 août 2013, à 16:44 +0100, Adam Spiers a écrit : > I just looked at this again in the light of my current work on > > https://github.com/crowbar/barclamp-provisioner/pull/177 > > ... > > Vincent Untz ([email protected]) wrote: > > Hi, > > > > I just submitted this pull request, which I guess is worth discussing > > here: > > https://github.com/crowbar/barclamp-crowbar/pull/589 > > > > Here's a copy and paste from the commit message: > > > > =================== > > Until now, when creating a proposal with some existing json, it was > > required that the json contained a full proposal definition. Two big > > downsides of this are: > > > > a) it's easy to break stuff by accident when changing a proposal > > b) if the schema changes to add new mandatory attributes, people can > > not simply re-use old json they had around; they have to update them > > > > This commit allows passing an incomplete json with only a subset of > > attributes. It will take the default json, and overwrite any attribute > > there with the ones from the incomplete json. > > > > Note that this is note using Chef::Mixin::DeepMerge::deep_merge! as a > > deep merge is also merging arrays in attribute values, which is > > something not desired here. The hash_only_merge! API is actually > > available in Chef 11 in Chef::Mixin::DeepMerge, so this is just a > > backport. > > Why is merging of arrays not desired? There are plenty of arrays > scattered around the JSON schema:
Because if the template has: "foo": [ 1 , 2 ] and your json file to merge with the template has: "foo": [ 7, 8 ] you don't expect the resulting json to be: "foo": [ 1, 2, 7, 8 ] You only expect to have [ 7, 8 ]. In more concrete terms, look at https://github.com/crowbar/barclamp-crowbar/blob/release/pebbles/master/chef/data_bags/crowbar/bc-template-crowbar.json#L9 and imagine we have "provisioner": [ "/var/lib/config/crowbar/provisioner.json" ] in the config file. You clearly don't expect to have a "default" in the resulting json. Now, if there are cases where merging arrays is desired, then we can add an option to make that possible. But I actually believe it's very unlikely. I was also considering removing the use of deep merge when saving nodes in favor of the hash_only_merge. But it's a bit risky, so didn't want to touch that :-) Cheers, Vincent -- Les gens heureux ne sont pas pressés. _______________________________________________ Crowbar mailing list [email protected] https://lists.us.dell.com/mailman/listinfo/crowbar For more information: http://crowbar.github.com/
