BROOKLYN-286: merge config keys map values #173
Conversation
|
Apologies that this is such a huge pull request :-) |
|
I think it worths putting Implement ConfigKey.typeInheritance changes in a separate PR. |
|
I have broken a few of these commits out into their own PRs, to allow parts of it to be reviewed + merged faster. Please comment on those smaller PRs - we can address comments there, and then rebase this larger PR.
|
pr173/rename confMapThing.obj Part of #173 - please review + merge here, to decrease the size of PR #173 ! The previous name caused problems because there is also a “confMapThing” of type MapConfigKey. That has special behaviour, where it looks up any config with that prefix - so it picked up any config defined against confMapThing.obj as well.
Pr173/yaml config tests Part of #173 - please review + merge here, to decrease the size of PR #173 ! Builds on #174 (the config rename is required), so please review + merge that first. This just tests the existing functionality. Note there are a couple of disabled tests, which demonstrate existing limitations.
pr173: Refactor stubbing of jclouds for tests Part of #173 - please review + merge here, to decrease the size of PR #173 ! Reason for refactoring is: 1. share the one impl of the stubbed ComputeServiceRegistry (previously we had two) 2. allow that to be re-used by other tests in PR #173, that location's templateOptions config values are correctly merged.
|
Jenkins failure is related: |
| } else if (val1.isAbsent()) { | ||
| return val2; | ||
| } else if (val1.isNull()) { | ||
| return val1; // an explicit null means an override; don't merge |
There was a problem hiding this comment.
Can't set null values in yaml, because we are using ImmutableXXX all over the place. Worth confirming with a test. If that's really the case we should think about other ways of overriding/fixing support for null values.
There was a problem hiding this comment.
Didn't notice you've already addressed this in a commit, ignore.
|
Now that those sub-PRs are merged (thanks @neykov ), I've rebased against master and fixed the missing license headers. |
|
I've also extracted the commit "JcloudsLocation: handle config null values" to its own PR so that it can be reviewed + merged in isolation: #178 |
| import com.google.common.collect.Iterables; | ||
| import com.google.common.collect.Sets; | ||
|
|
||
| public class CollectionMerger { |
There was a problem hiding this comment.
This should be marked @Beta.
|
Great changes @aledsage. Looking forward to using them in blueprints. |
Adds: * ConfigKey.getParentInheritance (deprecating getInheritance) * ConfigKey.getTypeInheritance (not yet implemented) * Adds MapConfigKey.Builder * Some SoftwareProcess config keys now set typeInheritance(MERGE) Breaking backwards-compatibility changes: * ConfigInheritance.isInherited return type changed (method was @beta) * VanillaSoftwareProcess keys changed to MapConfigKey, rather than ConfigKey<Map> * BasicConfigKey.Builder: generics changed, to allow sub-typing by MapConfigKey.Builder * AbstractStructuredConfigKey.subType field changed from public to protected Change of semantics: * Previously, when looking up entity config, the order of preference (in EntityConfigMap) was: 1. look up own config, using key 2. look up inherited config, using key 3. look up “own bag”, using key’s name 4. look up “inherited bag”, using key’s name. Now the order is (1), (3), (2), (4).
If passed templateOptions via an entity’s provisioning.properties and also have templateOptions in the location config, then merge them.
4 participants
No description provided.