[
https://issues.apache.org/jira/browse/BROOKLYN-286?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15312169#comment-15312169
]
ASF GitHub Bot commented on BROOKLYN-286:
-----------------------------------------
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/173#discussion_r65524668
--- Diff:
utils/common/src/main/java/org/apache/brooklyn/util/collections/CollectionMerger.java
---
@@ -0,0 +1,234 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.brooklyn.util.collections;
+
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Preconditions.checkNotNull;
+
+import java.math.BigDecimal;
+import java.math.BigInteger;
+import java.util.Date;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import org.apache.brooklyn.util.guava.Maybe;
+
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Iterables;
+import com.google.common.collect.Sets;
+
+public class CollectionMerger {
--- End diff --
This should be marked `@Beta`.
> 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)
