[ 
https://issues.apache.org/jira/browse/BROOKLYN-286?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15310811#comment-15310811
 ] 

ASF GitHub Bot commented on BROOKLYN-286:
-----------------------------------------

GitHub user aledsage opened a pull request:

    https://github.com/apache/brooklyn-server/pull/173

    BROOKLYN-286: merge config keys map values

    

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/aledsage/brooklyn-server merge-config

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/brooklyn-server/pull/173.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #173
    
----
commit 123ed23b9477188ff7580dfca0d6030ef4361f5b
Author: Aled Sage <[email protected]>
Date:   2016-05-26T09:39:52Z

    Rename TestEntity confMapThing.obj to confMapObjThing
    
    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.

commit 66e7b291c79acba125113bd41835397459a03916
Author: Aled Sage <[email protected]>
Date:   2016-05-26T09:40:37Z

    AbstractYamlTest.setUp/tearDown throws Exception

commit 14cc40b6ffc4c59425a29387ffda5c764a602c58
Author: Aled Sage <[email protected]>
Date:   2016-05-26T10:01:30Z

    Add YAML config tests

commit 27e43c119c9e10c78fd36483aed10c7d14d4225c
Author: Aled Sage <[email protected]>
Date:   2016-05-27T08:40:03Z

    Adds CollectionMerger, for merging maps

commit 67e904b31a770959a3082bb5945c0fb08b4d7a25
Author: Aled Sage <[email protected]>
Date:   2016-05-27T08:43:53Z

    Makes config parent-inheritance “merge” configurable
    
    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).

commit b94dd1015a6da86d82b2cbb5efc4bb2206a9124c
Author: Aled Sage <[email protected]>
Date:   2016-05-27T08:44:09Z

    Implement ConfigKey.typeInheritance

commit f9228c32ddf7ca66a0492ef5241d2990247ddd41
Author: Aled Sage <[email protected]>
Date:   2016-06-01T16:31:17Z

    Refactor stubbing of jclouds for tests

commit e65527c3ff76173882b335e0bb6c87604f01bd20
Author: Aled Sage <[email protected]>
Date:   2016-06-01T16:34:17Z

    Adds JcloudsLocation.LOOKUP_AWS_HOSTNAME

commit 09a5777e175286c82ec8dbc78672f2ff7d7dfb1f
Author: Aled Sage <[email protected]>
Date:   2016-06-01T16:37:48Z

    JcloudsLocation.templateOptions values merged
    
    If passed templateOptions via an entity’s provisioning.properties and
    also have templateOptions in the location config, then merge them.

commit fee4df6fadd006f1d3d36777e80f2e2ebc1b5793
Author: Aled Sage <[email protected]>
Date:   2016-06-01T16:57:50Z

    Disable EntityConfigTest.testGetConfigNonBlocking

----


> merge config keys map values, where appropriate
> -----------------------------------------------
>
>                 Key: BROOKLYN-286
>                 URL: https://issues.apache.org/jira/browse/BROOKLYN-286
>             Project: Brooklyn
>          Issue Type: New Feature
>    Affects Versions: 0.9.0
>            Reporter: Aled Sage
>
> See the email discussion on [email protected], subject "[PROPOSAL] 
> merging config keys", initially kicked off 25/05/2016, 12:12.
> Below is a copy of that initial proposal, which has then been further 
> discussed on the mailing list.
> TL;DR: we should merge config when overriding entities/locations, where it's 
> obvious that such behaviour is desired. For example, where an entity type 
> defines shell.env, then a new entity extending this type should inherit and 
> add to those values.
> _*REQUIREMENTS*_
> _*shell.env in entities*_
> When extending an existing entity type in YAML, it is not possible to extend 
> the set of environment variables. Instead, if the sub-type declares shell.env 
> it will override the inherited values.
> For example, consider the catalog items below:
>    # Catalog
>    brooklyn.catalog:
>       items:
>       - id: machine-with-env
>         item:
>           type:
>    org.apache.brooklyn.entity.software.base.VanillaSoftwareProcess
>           brooklyn.config:
>             shell.env:
>               ENV1: myEnv1
>    # Blueprint
>    location: ...
>    services:
>    - type: machine-with-env
>       brooklyn.config:
>         shell.env:
>           ENV2: myEnv2
>         launch.command: echo "ENV1=$ENV1, ENV2=$ENV2"
> A user might well expect the launch.command to have myEnv1 and myEnv2. 
> However, it does not get the ENV1 environment variable. This is a real pain 
> when trying to customize stock blueprints.
> We propose that the shell.env map should be *merged*.
> _*provisioning.properties*_
> An entity can be configured with provisioning.properties. These are passed to 
> the location when obtaining a new machine. They supplement and override the 
> values configured on the location. However, for templateOptions the 
> expected/desired behaviour would be to merge the options.
> Consider the blueprint below:_*
> *_
>    location:
>       minCores: 1
>       templateOptions:
>         networks: myNetwork
>    services:
>    - type: org.apache.brooklyn.entity.machine.MachineEntity
>       brooklyn.config:
>         provisioning.properties:
>           minRam: 2G
>           templateOptions:
>             tags: myTag
> A user might well expect the VM to be created with the given networks and 
> tags. However, currently the templateOptions in provisoining.properties will 
> override the existing value, rather than being merged with it.
> We propose that the templateOptions map should be *merged*.
> Valentin made a start to fix this in 
> https://github.com/apache/brooklyn-server/pull/151.
> _*_*provisioning.properties in sub-entities*_
> *_
> A similar argument holds for when extending an entity-type in YAML.
> If the super-type declares template options, then any additional 
> provisioning.properties declared on the entity sub-type should be *merged* 
> (including merging the templateOptions map contained within it).
> _*files.preinstall, templates.preinstall, etc*_
> The same applies for the map config for: files.preinstall, 
> templates.preinstall, files.install, templates.install, files.runtime and 
> templates.runtime.
> We propose that these maps get *merged* with the value defined in the 
> super-type.
> _*Overriding default values*_
> For default values in the super-type, we propose that this value *does* get 
> overridden, rather than merged.
> For example, in the blueprint below we suggest that the launch-command in the 
> sub-type should have ENV2 but not ENV_IN_DEFAULT.
>    brooklyn.catalog:
>       items:
>       - id: machine-with-env
>         version: 1.0.0
>         item:
>           type:
>    org.apache.brooklyn.entity.software.base.VanillaSoftwareProcess
>           brooklyn.parameters:
>           - name: shell.env
>             default:
>               ENV_IN_DEFAULT: myEnvInDefault
>       - id: machine-with-env-2
>         version: 1.0.0
>         item:
>           type: machine-with-env
>           brooklyn.config:
>             shell.env:
>               ENV2: myEnv2
>             launch.command: echo "ENV_IN_DEFAULT=$ENV_IN_DEFAULT,
>    ENV2=$ENV2"
> (Interestingly, the current behaviour of machine-with-env is that it gets the 
> value for ENV_IN_DEFAULT but not for ENV2, so sometime strange is going on 
> with re-defining the shell.env config key!)
> _*Extending commands: deferred*_
> Another scenario is where a super-type declares a value for 
> `install.command`, and the sub-type wants to augment this by adding 
> additional commands. Currently that is not possible. Instead the sub-type 
> needs to use pre.install.command and/or post.install.command. But that leads 
> to the same problem if a super-type also has a value defined for that key.
> Svet suggested we could perhaps introduce something like $brooklyn:super().
> Unless we can generalise that approach to also solve the merging of 
> `shell.env` etc, then I suggest we defer the `install.command` use-case. That 
> can be proposed and discussed in a different thread.
> However, if we can solve these problems with clever explicit use of 
> $brooklyn:super(), then that could provide an elegant solution to all of 
> these problems!
> _*Inheritance from parent entities*_
> Things are made yet more complicated by the fact we inherit config from 
> parent entities, in the entity hierarchy.
> We propose that this behaviour is also configurable for the config key, but 
> that the defaults stay as they are. The existing logic is applied to find the 
> config value that applies to the given entity. That value is then merged with 
> its super-type, as appropriate.
> For example, in the blueprint below... machine1 would get ENV1 and ENV2 (i.e. 
> the ENV1 definition overrides the ENV_IN_APP definition). However, machine2 
> would get ENV1 and ENV_IN_APP (i.e. it inherits ENV_IN_APP from the parent, 
> and this is meged with the super-type).
>    services:
>    - type: org.apache.brooklyn.entity.stock.BasicApplication
>       brooklyn.config:
>         shell.env:
>           ENV_IN_APP: myEnvInApp
>       brooklyn.children:
>       - type: machine-with-env
>         id: machine1
>         brooklyn.config:
>           shell.env:
>             ENV2: myEnv2
>       - type: machine-with-env
>         id: machine2
> The reasoning behind this is to figure out the inheritance/override rules 
> incrementally. We leave the parent-inheritance as-is, and just focus on the 
> sub-typing inheritance.
> Note that there is already a ConfigInheritance defined on ConfigKey for 
> controlling this kind of inheritance from the parent. The legal values for 
> ConfigInheritance are currently just ALWAYS and NONE.
> _*IMPLEMENTATION*_
> Clearly we do not want to implement this piecemeal. We'll add a way to 
> declare that a config key should be merged with that value from the 
> super-type.
> We'll change the Java ConfigKey code to be:
>    public interface ConfigKey {
>       /**
>        * @since 0.10.0
>        */
>       @Nullable ConfigInheritance getParentInheritance();
>       /**
>        * @since 0.10.0
>        */
>    @Nullable ConfigInheritance getTypeInheritance();
>       /**
>        * @deprecated since 0.10.0; instead use {@link
>    #getParentInheritance()}
>        */
>       @Nullable ConfigInheritance getInheritance();
>    }
> We'll add to ConfigInheritance support for MERGE. We'll change the name 
> "ALWAYS" to OVERRIDE (deprecating the old value).
> We'll change EntityConfigMap.getConfig to handle this new merge behaviour. 
> And same for locations, policies and enrichers.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to