You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@brooklyn.apache.org by he...@apache.org on 2015/04/17 16:35:02 UTC
[6/8] incubator-brooklyn git commit: addressing comments from code
review on enhancing yaml
addressing comments from code review on enhancing yaml
Project: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/commit/7ba6df70
Tree: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/tree/7ba6df70
Diff: http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/diff/7ba6df70
Branch: refs/heads/master
Commit: 7ba6df707b7485469c6c0d75cddab46bd849ccb9
Parents: f7142a3
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Authored: Wed Apr 15 20:31:19 2015 -0500
Committer: Alex Heneveld <al...@cloudsoftcorp.com>
Committed: Wed Apr 15 20:33:14 2015 -0500
----------------------------------------------------------------------
.../brooklyn/config/internal/AbstractConfigMapImpl.java | 7 ++++---
.../main/java/brooklyn/entity/basic/EntityConfigMap.java | 2 +-
core/src/main/java/brooklyn/event/basic/MapConfigKey.java | 10 ++++++++--
.../main/java/brooklyn/policy/basic/ConfigMapImpl.java | 2 +-
docs/guide/yaml/yaml-reference.md | 8 ++++++--
5 files changed, 20 insertions(+), 9 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/7ba6df70/core/src/main/java/brooklyn/config/internal/AbstractConfigMapImpl.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/config/internal/AbstractConfigMapImpl.java b/core/src/main/java/brooklyn/config/internal/AbstractConfigMapImpl.java
index 20f5812..defb5bc 100644
--- a/core/src/main/java/brooklyn/config/internal/AbstractConfigMapImpl.java
+++ b/core/src/main/java/brooklyn/config/internal/AbstractConfigMapImpl.java
@@ -64,7 +64,7 @@ public abstract class AbstractConfigMapImpl implements ConfigMap {
return getConfigRaw(key, true).orNull();
}
- protected Object setConfigPrep1(ConfigKey<?> key, Object v) {
+ protected Object coerceConfigVal(ConfigKey<?> key, Object v) {
Object val;
if ((v instanceof Future) || (v instanceof DeferredSupplier)) {
// no coercion for these (coerce on exit)
@@ -82,8 +82,9 @@ public abstract class AbstractConfigMapImpl implements ConfigMap {
val = TypeCoercions.coerce(v, key.getTypeToken());
} catch (Exception e) {
throw new IllegalArgumentException("Cannot coerce or set "+v+" to "+key, e);
- // if can't coerce, we could just log, and *throw* the error when we retrieve the config
- // but for now, fail fast (above)
+ // if can't coerce, we could just log as below and *throw* the error when we retrieve the config
+ // but for now, fail fast (above), because we haven't encountered strong use cases
+ // where we want to do coercion on retrieval, except for the exceptions above
// Exceptions.propagateIfFatal(e);
// LOG.warn("Cannot coerce or set "+v+" to "+key+" (ignoring): "+e, e);
// val = v;
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/7ba6df70/core/src/main/java/brooklyn/entity/basic/EntityConfigMap.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/entity/basic/EntityConfigMap.java b/core/src/main/java/brooklyn/entity/basic/EntityConfigMap.java
index 3efe8fa..f2821d8 100644
--- a/core/src/main/java/brooklyn/entity/basic/EntityConfigMap.java
+++ b/core/src/main/java/brooklyn/entity/basic/EntityConfigMap.java
@@ -187,7 +187,7 @@ public class EntityConfigMap extends AbstractConfigMapImpl {
@SuppressWarnings("unchecked")
public Object setConfig(ConfigKey<?> key, Object v) {
- Object val = setConfigPrep1(key, v);
+ Object val = coerceConfigVal(key, v);
Object oldVal;
if (key instanceof StructuredConfigKey) {
oldVal = ((StructuredConfigKey)key).applyValueToMap(val, ownConfig);
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/7ba6df70/core/src/main/java/brooklyn/event/basic/MapConfigKey.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/event/basic/MapConfigKey.java b/core/src/main/java/brooklyn/event/basic/MapConfigKey.java
index c682b3b..811aabb 100644
--- a/core/src/main/java/brooklyn/event/basic/MapConfigKey.java
+++ b/core/src/main/java/brooklyn/event/basic/MapConfigKey.java
@@ -22,6 +22,7 @@ import java.util.Collections;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.Map.Entry;
+import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.ExecutionException;
import org.slf4j.Logger;
@@ -132,15 +133,20 @@ public class MapConfigKey<V> extends AbstractStructuredConfigKey<Map<String,V>,M
} else {
// supplier or other unexpected value
if (k instanceof Supplier) {
- // TODO not thread-safe
Object mapAtRoot = target.get(this);
if (mapAtRoot==null) {
mapAtRoot = new LinkedHashMap();
target.put(this, mapAtRoot);
}
+ // TODO above is not thread-safe, and below is assuming synching on map
+ // is the best way to prevent CME's, which is often but not always true
if (mapAtRoot instanceof Map) {
- synchronized (mapAtRoot) {
+ if (mapAtRoot instanceof ConcurrentMap) {
return ((Map)mapAtRoot).put(k, value.getValue());
+ } else {
+ synchronized (mapAtRoot) {
+ return ((Map)mapAtRoot).put(k, value.getValue());
+ }
}
}
}
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/7ba6df70/core/src/main/java/brooklyn/policy/basic/ConfigMapImpl.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/brooklyn/policy/basic/ConfigMapImpl.java b/core/src/main/java/brooklyn/policy/basic/ConfigMapImpl.java
index c438fef..dc34b17 100644
--- a/core/src/main/java/brooklyn/policy/basic/ConfigMapImpl.java
+++ b/core/src/main/java/brooklyn/policy/basic/ConfigMapImpl.java
@@ -105,7 +105,7 @@ public class ConfigMapImpl extends AbstractConfigMapImpl {
}
public Object setConfig(ConfigKey<?> key, Object v) {
- Object val = setConfigPrep1(key, v);
+ Object val = coerceConfigVal(key, v);
if (key instanceof StructuredConfigKey) {
return ((StructuredConfigKey)key).applyValueToMap(val, ownConfig);
} else {
http://git-wip-us.apache.org/repos/asf/incubator-brooklyn/blob/7ba6df70/docs/guide/yaml/yaml-reference.md
----------------------------------------------------------------------
diff --git a/docs/guide/yaml/yaml-reference.md b/docs/guide/yaml/yaml-reference.md
index 71993ae..64c81a5 100644
--- a/docs/guide/yaml/yaml-reference.md
+++ b/docs/guide/yaml/yaml-reference.md
@@ -146,13 +146,15 @@ Dependency injection other powerful references and types can be built up within
concise DSL defined here:
* `$brooklyn:attributeWhenReady("sensor")` will store a future which will be blocked when it is accessed,
- until the given `sensor` from the component `ID` has a "truthy" (i.e. non-trivial, non-empty, non-zero) value
+ until the given `sensor` from this entity "truthy" (i.e. non-trivial, non-empty, non-zero) value
+ (see below on `component` for looking up values on other sensors)
* `$brooklyn:config("key")` will insert the value set against the given key at this entity (or nearest ancestor);
can be used to supply config at the root which is used in multiple places in the plan
* `$brooklyn:sensor("sensor.name")` returns the given sensor on the current entity if found, or an untyped (Object) sensor;
`$brooklyn:sensor("io.brooklyn.ContainingEntityClass", "sensor.name")` returns the strongly typed sensor defined in the given class
* `$brooklyn:component("ID")` refers to a Brooklyn component with the given ID; you can then access the following subfields,
- using the same syntax as defined above but with a different reference entity:
+ using the same syntax as defined above but with a different reference entity,
+ e.g. `$brooklyn:component("ID").attributeWhenReady("sensor")`:
* `.attributeWhenReady("sensor")`
* `.config("key")`
* `.sensor("sensor.name")`
@@ -172,6 +174,8 @@ concise DSL defined here:
but as an `EntitySpec` suitable for setting as the value of `ConfigKey<EntitySpec>` config items
(such as `memberSpec` in `DynamicCluster`)
+<!-- TODO examples for object and entitySpec -->
+
Parameters above can be supplied either as strings or as lists and maps in YAML,
and the `$brooklyn:` syntax can be used within those parameters.