[GitHub] brooklyn-server pull request #971: DSL for $brooklyn:location()

2018-09-04 Thread asfgit
Github user asfgit closed the pull request at:

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


---


[GitHub] brooklyn-server pull request #971: DSL for $brooklyn:location()

2018-09-04 Thread nakomis
Github user nakomis commented on a diff in the pull request:

https://github.com/apache/brooklyn-server/pull/971#discussion_r214940310
  
--- Diff: 
camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/DslAndRebindYamlTest.java
 ---
@@ -467,6 +467,30 @@ public void testDslLocation() throws Exception {
 assertEquals(getConfigInTask(testEntity, 
ConfigKeys.newConfigKey(Object.class, "config2")), appLoc);
 }
 
+@Test
+public void testDslLocationIndexOutOfBounds() throws Exception {
+String yaml = Joiner.on("\n").join(
+"location: localhost",
+"services:",
+"- type: " + BasicApplication.class.getName(),
+"  brooklyn.config:",
+"config1: $brooklyn:location(\"1\")");
+
+Application app = (Application) 
createStartWaitAndLogApplication(yaml);
+
+Location appLoc = Iterables.getOnlyElement(app.getLocations());
+try {
+assertEquals(getConfigInTask(app, 
ConfigKeys.newConfigKey(Object.class, "config1")), appLoc);
--- End diff --

If an exception is not thrown here the test will pass when it should fail. 
This case should be checked, e.g. by throwing `IllegalStateException` after 
this line


---


[GitHub] brooklyn-server pull request #971: DSL for $brooklyn:location()

2018-06-18 Thread geomacy
Github user geomacy commented on a diff in the pull request:

https://github.com/apache/brooklyn-server/pull/971#discussion_r196149356
  
--- Diff: 
camp/camp-brooklyn/src/main/java/org/apache/brooklyn/camp/brooklyn/spi/dsl/methods/DslComponent.java
 ---
@@ -737,6 +720,130 @@ public String toString() {
 }
 }
 
+@DslAccessible
+public BrooklynDslDeferredSupplier location() {
+return new DslLocationSupplier(this, 0);
+}
+
+@DslAccessible
+public BrooklynDslDeferredSupplier location(Object index) {
+return new DslLocationSupplier(this, index);
+}
+
+protected final static class DslLocationSupplier extends 
BrooklynDslDeferredSupplier {
+private static final long serialVersionUID = 5597335296158584040L;
+private final DslComponent component;
+private final Object index;
+
+public DslLocationSupplier(DslComponent component, Object index) {
+this.component = Preconditions.checkNotNull(component);
+this.index = index;
+}
+
+@Override
+public final Maybe getImmediately() {
+Callable job = new Callable() {
+@Override public Object call() {
+Maybe targetEntityMaybe = 
component.getImmediately();
+if (targetEntityMaybe.isAbsent()) return 
ImmediateValueNotAvailableException.newAbsentWrapping("Target entity not 
available: "+component, targetEntityMaybe);
+Entity targetEntity = targetEntityMaybe.get();
+
+int indexI = resolveIndex(true);
+
+Collection locations = 
getLocations(targetEntity);
+if (locations.isEmpty()) {
+throw new 
ImmediateValueNotAvailableException("Target entity has no locations: 
"+component);
+} else if (locations.size() < (indexI + 1)) {
+throw new IndexOutOfBoundsException("Target entity 
("+component+") has "+locations.size()+" location(s), but requested index 
"+index);
+}
+Location result = Iterables.get(locations, indexI);
+if (result == null) {
+throw new NullPointerException("Target entity 
("+component+") has null location at index "+index);
+}
+return result;
+}
+};
+
+return findExecutionContext(this).getImmediately(job);
+}
+
+// Pattern copied from DslConfigSupplier; see that for explanation
+@Override
+public Task newTask() {
+boolean immediate = false;
+
+Callable job = new Callable() {
+@Override
+public Object call() throws Exception {
+Entity targetEntity = component.get();
+
+// this is always run in a new dedicated task 
(possibly a fake task if immediate), so no need to clear
+checkAndTagForRecursiveReference(targetEntity);
+
+int indexI = resolveIndex(immediate);
+
+// TODO Try repeatedly if no location(s)?
+Collection locations = 
getLocations(targetEntity);
+if (locations.size() < (indexI + 1)) {
+throw new IndexOutOfBoundsException("Target entity 
("+component+") has "+locations.size()+" location(s), but requested index 
"+index);
--- End diff --

The behaviour is perhaps a bit odd from the app point of view. Say you set 
a config value to a location with a bad index,  the exception is logged in 
`brooklyn.debug.log` (not `info`), but the config value is still rendered in 
the application, which doesn't go on fire.  The value isn't a location of 
course, but the supplier:

```
$ br app bn ent BEN config
Key| Value
camp.template.id   | yJlI9Z2t
testLocN   | map[component:map[componentId: 
componentIdSupplier: scopeComponent: scope:THIS] index:3]
brooklyn.wrapper_app   | true
```
which users may find confusing?


---


[GitHub] brooklyn-server pull request #971: DSL for $brooklyn:location()

2018-06-08 Thread aledsage
GitHub user aledsage opened a pull request:

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

DSL for $brooklyn:location()



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

$ git pull https://github.com/aledsage/brooklyn-server dsl-location

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

https://github.com/apache/brooklyn-server/pull/971.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 #971


commit 6b157a307477013010f436904af6abe18610e3c1
Author: Aled Sage 
Date:   2018-06-06T16:34:39Z

Add support for $brooklyn:location()

commit c73a48a5c663b1395aeb0fb34eea58660bbad021
Author: Aled Sage 
Date:   2018-06-07T21:12:06Z

DslComponent: code tidy




---