You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by ahgittin <gi...@git.apache.org> on 2017/09/13 08:30:07 UTC
[GitHub] brooklyn-server pull request #818: More on adjunct highlights
GitHub user ahgittin opened a pull request:
https://github.com/apache/brooklyn-server/pull/818
More on adjunct highlights
Applies #813 to a policy (`ServiceRestarter`) to see how it works. As reported there it works good, REST and persistence acting as expected.
The API for policy authors was a little awkward, and there were some PR review comments in #813. This addresses both of these.
I will be adding highlight info to more policies and adjuncts. If this is reviewed in time I'll do it in a different branch, else I'll add them here for simplicity.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/ahgittin/brooklyn-server highlights-adjuncts
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/brooklyn-server/pull/818.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 #818
----
commit 1a8df4275d61ddd69e6bc05b057c081dd4dcad34
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2017-09-13T08:07:44Z
use highlights in `ServiceRestarter`
commit 5ae9c41c1dc8d2884598a2375581d2c4dc14ce14
Author: Alex Heneveld <al...@cloudsoftcorp.com>
Date: 2017-09-13T08:24:09Z
expand API for policy java authors, and address PR comments incl javadoc
see ServiceRestarter for example of how the API can be used by authors now
----
---
[GitHub] brooklyn-server pull request #818: More on adjunct highlights
Posted by Graeme-Miller <gi...@git.apache.org>.
Github user Graeme-Miller commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/818#discussion_r138642216
--- Diff: core/src/main/java/org/apache/brooklyn/core/objs/AbstractEntityAdjunct.java ---
@@ -568,6 +570,98 @@ public void setUniqueTag(String uniqueTag) {
return highlightsToReturn;
}
+ /** Records a named highlight against this object, for persistence and API access.
+ * See common highlights including {@link #HIGHLIGHT_NAME_LAST_ACTION} and
+ * {@link #HIGHLIGHT_NAME_LAST_CONFIRMATION}.
+ * Also see convenience methods eg {@link #highlightOngoing(String, String)} and {@link #highlight(String, String, Task)}
+ * and {@link HighlightTuple}.
+ */
+ protected void setHighlight(String name, HighlightTuple tuple) {
+ highlights.put(name, tuple);
+ }
+
+ /** As {@link #setHighlight(String, HighlightTuple)}, convenience for recording an item which is intended to be ongoing. */
+ protected void highlightOngoing(String name, String description) {
+ highlights.put(name, new HighlightTuple(description, 0, null));
--- End diff --
Should be calling setHighlight, rather than putting directly into the map. This would allow these methods to remain uncoupled from how we store the highlights. Same for the below highlight methods.
---
[GitHub] brooklyn-server pull request #818: More on adjunct highlights
Posted by grkvlt <gi...@git.apache.org>.
Github user grkvlt commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/818#discussion_r138919622
--- Diff: core/src/main/java/org/apache/brooklyn/core/objs/AbstractEntityAdjunct.java ---
@@ -568,6 +570,98 @@ public void setUniqueTag(String uniqueTag) {
return highlightsToReturn;
}
+ /** Records a named highlight against this object, for persistence and API access.
+ * See common highlights including {@link #HIGHLIGHT_NAME_LAST_ACTION} and
+ * {@link #HIGHLIGHT_NAME_LAST_CONFIRMATION}.
+ * Also see convenience methods eg {@link #highlightOngoing(String, String)} and {@link #highlight(String, String, Task)}
+ * and {@link HighlightTuple}.
+ */
+ protected void setHighlight(String name, HighlightTuple tuple) {
+ highlights.put(name, tuple);
+ }
+
+ /** As {@link #setHighlight(String, HighlightTuple)}, convenience for recording an item which is intended to be ongoing. */
+ protected void highlightOngoing(String name, String description) {
+ highlights.put(name, new HighlightTuple(description, 0, null));
+ }
+ /** As {@link #setHighlight(String, HighlightTuple)}, convenience for recording an item with the current time. */
+ protected void highlightNow(String name, String description) {
+ highlights.put(name, new HighlightTuple(description, System.currentTimeMillis(), null));
+ }
+ /** As {@link #setHighlight(String, HighlightTuple)}, convenience for recording an item with the current time and given task. */
+ protected void highlight(String name, String description, @Nullable Task<?> t) {
+ highlights.put(name, new HighlightTuple(description, System.currentTimeMillis(), t!=null ? t.getId() : null));
+ }
+
+ /** As {@link #setHighlight(String, HighlightTuple)} for {@link #HIGHLIGHT_NAME_TRIGGERS} (as ongoing). */
+ protected void highlightTriggers(String description) {
+ highlightOngoing(HIGHLIGHT_NAME_TRIGGERS, description);
+ }
+ protected <T> void highlightTriggers(Sensor<?> s, Object source) {
+ highlightTriggers(Collections.singleton(s), Collections.singleton(source));
+ }
+ protected <T> void highlightTriggers(Iterable<? extends Sensor<? extends T>> s, Object source) {
+ highlightTriggers(s, (Iterable<?>) (source instanceof Iterable ? (Iterable<?>)source : Collections.singleton(source)));
+ }
+ protected <U> void highlightTriggers(Sensor<?> s, Iterable<U> sources) {
+ highlightTriggers(Collections.singleton(s), sources);
+ }
+ protected <T,U> void highlightTriggers(Iterable<? extends Sensor<? extends T>> sensors, Iterable<U> sources) {
+ StringBuilder msg = new StringBuilder("Listening for ");
+ boolean firstWord = true;
+ if (sensors!=null) for (Object s: sensors) {
+ if (s==null) continue;
+ if (!firstWord) { msg.append(", "); } else { firstWord = false; }
+ // s is normally a sensor but forgive other things if caller cheated generics
+ msg.append(s instanceof Sensor ? ((Sensor<?>)s).getName() : s.toString());
+ }
+ if (firstWord) msg.append("<nothing>");
+
+ firstWord = true;
--- End diff --
Looks nice @ahgittin - it's taken me a while to get used to operating in this style with Java as well, but it does makes things both concise and still readable...
---
[GitHub] brooklyn-server pull request #818: More on adjunct highlights
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/818#discussion_r138815073
--- Diff: core/src/main/java/org/apache/brooklyn/core/objs/AbstractEntityAdjunct.java ---
@@ -568,6 +570,98 @@ public void setUniqueTag(String uniqueTag) {
return highlightsToReturn;
}
+ /** Records a named highlight against this object, for persistence and API access.
+ * See common highlights including {@link #HIGHLIGHT_NAME_LAST_ACTION} and
+ * {@link #HIGHLIGHT_NAME_LAST_CONFIRMATION}.
+ * Also see convenience methods eg {@link #highlightOngoing(String, String)} and {@link #highlight(String, String, Task)}
+ * and {@link HighlightTuple}.
+ */
+ protected void setHighlight(String name, HighlightTuple tuple) {
+ highlights.put(name, tuple);
+ }
+
+ /** As {@link #setHighlight(String, HighlightTuple)}, convenience for recording an item which is intended to be ongoing. */
+ protected void highlightOngoing(String name, String description) {
+ highlights.put(name, new HighlightTuple(description, 0, null));
--- End diff --
agree
---
[GitHub] brooklyn-server pull request #818: More on adjunct highlights
Posted by Graeme-Miller <gi...@git.apache.org>.
Github user Graeme-Miller commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/818#discussion_r138642085
--- Diff: core/src/main/java/org/apache/brooklyn/core/objs/AbstractEntityAdjunct.java ---
@@ -568,6 +570,98 @@ public void setUniqueTag(String uniqueTag) {
return highlightsToReturn;
}
+ /** Records a named highlight against this object, for persistence and API access.
+ * See common highlights including {@link #HIGHLIGHT_NAME_LAST_ACTION} and
+ * {@link #HIGHLIGHT_NAME_LAST_CONFIRMATION}.
+ * Also see convenience methods eg {@link #highlightOngoing(String, String)} and {@link #highlight(String, String, Task)}
+ * and {@link HighlightTuple}.
+ */
+ protected void setHighlight(String name, HighlightTuple tuple) {
+ highlights.put(name, tuple);
+ }
+
+ /** As {@link #setHighlight(String, HighlightTuple)}, convenience for recording an item which is intended to be ongoing. */
+ protected void highlightOngoing(String name, String description) {
+ highlights.put(name, new HighlightTuple(description, 0, null));
+ }
+ /** As {@link #setHighlight(String, HighlightTuple)}, convenience for recording an item with the current time. */
+ protected void highlightNow(String name, String description) {
+ highlights.put(name, new HighlightTuple(description, System.currentTimeMillis(), null));
+ }
+ /** As {@link #setHighlight(String, HighlightTuple)}, convenience for recording an item with the current time and given task. */
+ protected void highlight(String name, String description, @Nullable Task<?> t) {
+ highlights.put(name, new HighlightTuple(description, System.currentTimeMillis(), t!=null ? t.getId() : null));
+ }
+
+ /** As {@link #setHighlight(String, HighlightTuple)} for {@link #HIGHLIGHT_NAME_TRIGGERS} (as ongoing). */
+ protected void highlightTriggers(String description) {
+ highlightOngoing(HIGHLIGHT_NAME_TRIGGERS, description);
+ }
+ protected <T> void highlightTriggers(Sensor<?> s, Object source) {
+ highlightTriggers(Collections.singleton(s), Collections.singleton(source));
+ }
+ protected <T> void highlightTriggers(Iterable<? extends Sensor<? extends T>> s, Object source) {
+ highlightTriggers(s, (Iterable<?>) (source instanceof Iterable ? (Iterable<?>)source : Collections.singleton(source)));
+ }
+ protected <U> void highlightTriggers(Sensor<?> s, Iterable<U> sources) {
+ highlightTriggers(Collections.singleton(s), sources);
+ }
+ protected <T,U> void highlightTriggers(Iterable<? extends Sensor<? extends T>> sensors, Iterable<U> sources) {
--- End diff --
This method would benefit from comments to explain what it is doing, with multiple loops and nested ifs it isn't easy to tell at a glance.
---
[GitHub] brooklyn-server pull request #818: More on adjunct highlights
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/818#discussion_r138815798
--- Diff: core/src/main/java/org/apache/brooklyn/core/objs/AbstractEntityAdjunct.java ---
@@ -568,6 +570,98 @@ public void setUniqueTag(String uniqueTag) {
return highlightsToReturn;
}
+ /** Records a named highlight against this object, for persistence and API access.
+ * See common highlights including {@link #HIGHLIGHT_NAME_LAST_ACTION} and
+ * {@link #HIGHLIGHT_NAME_LAST_CONFIRMATION}.
+ * Also see convenience methods eg {@link #highlightOngoing(String, String)} and {@link #highlight(String, String, Task)}
+ * and {@link HighlightTuple}.
+ */
+ protected void setHighlight(String name, HighlightTuple tuple) {
+ highlights.put(name, tuple);
+ }
+
+ /** As {@link #setHighlight(String, HighlightTuple)}, convenience for recording an item which is intended to be ongoing. */
+ protected void highlightOngoing(String name, String description) {
+ highlights.put(name, new HighlightTuple(description, 0, null));
+ }
+ /** As {@link #setHighlight(String, HighlightTuple)}, convenience for recording an item with the current time. */
+ protected void highlightNow(String name, String description) {
+ highlights.put(name, new HighlightTuple(description, System.currentTimeMillis(), null));
+ }
+ /** As {@link #setHighlight(String, HighlightTuple)}, convenience for recording an item with the current time and given task. */
+ protected void highlight(String name, String description, @Nullable Task<?> t) {
+ highlights.put(name, new HighlightTuple(description, System.currentTimeMillis(), t!=null ? t.getId() : null));
+ }
+
+ /** As {@link #setHighlight(String, HighlightTuple)} for {@link #HIGHLIGHT_NAME_TRIGGERS} (as ongoing). */
+ protected void highlightTriggers(String description) {
+ highlightOngoing(HIGHLIGHT_NAME_TRIGGERS, description);
+ }
+ protected <T> void highlightTriggers(Sensor<?> s, Object source) {
+ highlightTriggers(Collections.singleton(s), Collections.singleton(source));
+ }
+ protected <T> void highlightTriggers(Iterable<? extends Sensor<? extends T>> s, Object source) {
+ highlightTriggers(s, (Iterable<?>) (source instanceof Iterable ? (Iterable<?>)source : Collections.singleton(source)));
+ }
+ protected <U> void highlightTriggers(Sensor<?> s, Iterable<U> sources) {
+ highlightTriggers(Collections.singleton(s), sources);
+ }
+ protected <T,U> void highlightTriggers(Iterable<? extends Sensor<? extends T>> sensors, Iterable<U> sources) {
+ StringBuilder msg = new StringBuilder("Listening for ");
+ boolean firstWord = true;
+ if (sensors!=null) for (Object s: sensors) {
--- End diff --
clearly bad style, no argument! i added the guard when debugging, forgot to format it nicely...
---
[GitHub] brooklyn-server pull request #818: More on adjunct highlights
Posted by Graeme-Miller <gi...@git.apache.org>.
Github user Graeme-Miller commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/818#discussion_r138817948
--- Diff: core/src/main/java/org/apache/brooklyn/core/objs/AbstractEntityAdjunct.java ---
@@ -568,6 +570,98 @@ public void setUniqueTag(String uniqueTag) {
return highlightsToReturn;
}
+ /** Records a named highlight against this object, for persistence and API access.
+ * See common highlights including {@link #HIGHLIGHT_NAME_LAST_ACTION} and
+ * {@link #HIGHLIGHT_NAME_LAST_CONFIRMATION}.
+ * Also see convenience methods eg {@link #highlightOngoing(String, String)} and {@link #highlight(String, String, Task)}
+ * and {@link HighlightTuple}.
+ */
+ protected void setHighlight(String name, HighlightTuple tuple) {
+ highlights.put(name, tuple);
+ }
+
+ /** As {@link #setHighlight(String, HighlightTuple)}, convenience for recording an item which is intended to be ongoing. */
+ protected void highlightOngoing(String name, String description) {
+ highlights.put(name, new HighlightTuple(description, 0, null));
+ }
+ /** As {@link #setHighlight(String, HighlightTuple)}, convenience for recording an item with the current time. */
+ protected void highlightNow(String name, String description) {
+ highlights.put(name, new HighlightTuple(description, System.currentTimeMillis(), null));
+ }
+ /** As {@link #setHighlight(String, HighlightTuple)}, convenience for recording an item with the current time and given task. */
+ protected void highlight(String name, String description, @Nullable Task<?> t) {
+ highlights.put(name, new HighlightTuple(description, System.currentTimeMillis(), t!=null ? t.getId() : null));
+ }
+
+ /** As {@link #setHighlight(String, HighlightTuple)} for {@link #HIGHLIGHT_NAME_TRIGGERS} (as ongoing). */
+ protected void highlightTriggers(String description) {
+ highlightOngoing(HIGHLIGHT_NAME_TRIGGERS, description);
+ }
+ protected <T> void highlightTriggers(Sensor<?> s, Object source) {
+ highlightTriggers(Collections.singleton(s), Collections.singleton(source));
+ }
+ protected <T> void highlightTriggers(Iterable<? extends Sensor<? extends T>> s, Object source) {
+ highlightTriggers(s, (Iterable<?>) (source instanceof Iterable ? (Iterable<?>)source : Collections.singleton(source)));
+ }
+ protected <U> void highlightTriggers(Sensor<?> s, Iterable<U> sources) {
+ highlightTriggers(Collections.singleton(s), sources);
+ }
+ protected <T,U> void highlightTriggers(Iterable<? extends Sensor<? extends T>> sensors, Iterable<U> sources) {
+ StringBuilder msg = new StringBuilder("Listening for ");
+ boolean firstWord = true;
+ if (sensors!=null) for (Object s: sensors) {
+ if (s==null) continue;
+ if (!firstWord) { msg.append(", "); } else { firstWord = false; }
+ // s is normally a sensor but forgive other things if caller cheated generics
+ msg.append(s instanceof Sensor ? ((Sensor<?>)s).getName() : s.toString());
+ }
+ if (firstWord) msg.append("<nothing>");
+
+ firstWord = true;
--- End diff --
@grkvlt very interesting, didn't know about Collectors.joining(", "), that's very handy.
re. the second clause in the if. My understanding of Alex's code is that, if there is only one entry, and that entry is self, then we shouldn't add "on...". The second clause in the if is to guard against that. Maybe Alex can clarify?
---
[GitHub] brooklyn-server pull request #818: More on adjunct highlights
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/818#discussion_r138817982
--- Diff: core/src/main/java/org/apache/brooklyn/core/objs/AbstractEntityAdjunct.java ---
@@ -568,6 +570,98 @@ public void setUniqueTag(String uniqueTag) {
return highlightsToReturn;
}
+ /** Records a named highlight against this object, for persistence and API access.
+ * See common highlights including {@link #HIGHLIGHT_NAME_LAST_ACTION} and
+ * {@link #HIGHLIGHT_NAME_LAST_CONFIRMATION}.
+ * Also see convenience methods eg {@link #highlightOngoing(String, String)} and {@link #highlight(String, String, Task)}
+ * and {@link HighlightTuple}.
+ */
+ protected void setHighlight(String name, HighlightTuple tuple) {
+ highlights.put(name, tuple);
+ }
+
+ /** As {@link #setHighlight(String, HighlightTuple)}, convenience for recording an item which is intended to be ongoing. */
+ protected void highlightOngoing(String name, String description) {
+ highlights.put(name, new HighlightTuple(description, 0, null));
+ }
+ /** As {@link #setHighlight(String, HighlightTuple)}, convenience for recording an item with the current time. */
+ protected void highlightNow(String name, String description) {
+ highlights.put(name, new HighlightTuple(description, System.currentTimeMillis(), null));
+ }
+ /** As {@link #setHighlight(String, HighlightTuple)}, convenience for recording an item with the current time and given task. */
+ protected void highlight(String name, String description, @Nullable Task<?> t) {
+ highlights.put(name, new HighlightTuple(description, System.currentTimeMillis(), t!=null ? t.getId() : null));
+ }
+
+ /** As {@link #setHighlight(String, HighlightTuple)} for {@link #HIGHLIGHT_NAME_TRIGGERS} (as ongoing). */
+ protected void highlightTriggers(String description) {
+ highlightOngoing(HIGHLIGHT_NAME_TRIGGERS, description);
+ }
+ protected <T> void highlightTriggers(Sensor<?> s, Object source) {
+ highlightTriggers(Collections.singleton(s), Collections.singleton(source));
+ }
+ protected <T> void highlightTriggers(Iterable<? extends Sensor<? extends T>> s, Object source) {
+ highlightTriggers(s, (Iterable<?>) (source instanceof Iterable ? (Iterable<?>)source : Collections.singleton(source)));
+ }
+ protected <U> void highlightTriggers(Sensor<?> s, Iterable<U> sources) {
+ highlightTriggers(Collections.singleton(s), sources);
+ }
+ protected <T,U> void highlightTriggers(Iterable<? extends Sensor<? extends T>> sensors, Iterable<U> sources) {
+ StringBuilder msg = new StringBuilder("Listening for ");
+ boolean firstWord = true;
+ if (sensors!=null) for (Object s: sensors) {
+ if (s==null) continue;
+ if (!firstWord) { msg.append(", "); } else { firstWord = false; }
+ // s is normally a sensor but forgive other things if caller cheated generics
+ msg.append(s instanceof Sensor ? ((Sensor<?>)s).getName() : s.toString());
+ }
+ if (firstWord) msg.append("<nothing>");
+
+ firstWord = true;
--- End diff --
great suggestions -- so used to using lambas in JS but not in java :)
using choice operator to make more concise, going with:
```
if (sensors==null || Iterables.isEmpty(sensors)) {
msg.append("<nothing>");
} else {
String sensorsText = StreamSupport.stream(sensors.spliterator(), false)
.filter(s -> s != null)
.map(s -> (s instanceof Sensor ? ((Sensor<?>) s).getName() : s.toString()))
.collect(Collectors.joining(", "));
msg.append(sensorsText);
}
if (sources!=null && !Iterables.isEmpty(sources)) {
String sourcesText = StreamSupport.stream(sources.spliterator(), false)
.filter(s -> s != null)
.map(s -> (s.equals(getEntity()) ? "self" : s.toString()))
.collect(Collectors.joining(", "));
if (!"self".equals(sourcesText)) {
msg.append(" on ").append(sourcesText);
}
}
```
---
[GitHub] brooklyn-server pull request #818: More on adjunct highlights
Posted by m4rkmckenna <gi...@git.apache.org>.
Github user m4rkmckenna commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/818#discussion_r138696197
--- Diff: core/src/main/java/org/apache/brooklyn/core/objs/AbstractEntityAdjunct.java ---
@@ -568,6 +570,98 @@ public void setUniqueTag(String uniqueTag) {
return highlightsToReturn;
}
+ /** Records a named highlight against this object, for persistence and API access.
+ * See common highlights including {@link #HIGHLIGHT_NAME_LAST_ACTION} and
+ * {@link #HIGHLIGHT_NAME_LAST_CONFIRMATION}.
+ * Also see convenience methods eg {@link #highlightOngoing(String, String)} and {@link #highlight(String, String, Task)}
+ * and {@link HighlightTuple}.
+ */
+ protected void setHighlight(String name, HighlightTuple tuple) {
+ highlights.put(name, tuple);
+ }
+
+ /** As {@link #setHighlight(String, HighlightTuple)}, convenience for recording an item which is intended to be ongoing. */
+ protected void highlightOngoing(String name, String description) {
+ highlights.put(name, new HighlightTuple(description, 0, null));
+ }
+ /** As {@link #setHighlight(String, HighlightTuple)}, convenience for recording an item with the current time. */
+ protected void highlightNow(String name, String description) {
+ highlights.put(name, new HighlightTuple(description, System.currentTimeMillis(), null));
+ }
+ /** As {@link #setHighlight(String, HighlightTuple)}, convenience for recording an item with the current time and given task. */
+ protected void highlight(String name, String description, @Nullable Task<?> t) {
+ highlights.put(name, new HighlightTuple(description, System.currentTimeMillis(), t!=null ? t.getId() : null));
+ }
+
+ /** As {@link #setHighlight(String, HighlightTuple)} for {@link #HIGHLIGHT_NAME_TRIGGERS} (as ongoing). */
+ protected void highlightTriggers(String description) {
+ highlightOngoing(HIGHLIGHT_NAME_TRIGGERS, description);
+ }
+ protected <T> void highlightTriggers(Sensor<?> s, Object source) {
+ highlightTriggers(Collections.singleton(s), Collections.singleton(source));
+ }
+ protected <T> void highlightTriggers(Iterable<? extends Sensor<? extends T>> s, Object source) {
+ highlightTriggers(s, (Iterable<?>) (source instanceof Iterable ? (Iterable<?>)source : Collections.singleton(source)));
+ }
+ protected <U> void highlightTriggers(Sensor<?> s, Iterable<U> sources) {
+ highlightTriggers(Collections.singleton(s), sources);
+ }
+ protected <T,U> void highlightTriggers(Iterable<? extends Sensor<? extends T>> sensors, Iterable<U> sources) {
+ StringBuilder msg = new StringBuilder("Listening for ");
+ boolean firstWord = true;
+ if (sensors!=null) for (Object s: sensors) {
--- End diff --
Agree for the sake of readability please add braces and a line break
---
[GitHub] brooklyn-server pull request #818: More on adjunct highlights
Posted by Graeme-Miller <gi...@git.apache.org>.
Github user Graeme-Miller commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/818#discussion_r138638872
--- Diff: core/src/main/java/org/apache/brooklyn/core/objs/AbstractEntityAdjunct.java ---
@@ -568,6 +570,98 @@ public void setUniqueTag(String uniqueTag) {
return highlightsToReturn;
}
+ /** Records a named highlight against this object, for persistence and API access.
+ * See common highlights including {@link #HIGHLIGHT_NAME_LAST_ACTION} and
+ * {@link #HIGHLIGHT_NAME_LAST_CONFIRMATION}.
+ * Also see convenience methods eg {@link #highlightOngoing(String, String)} and {@link #highlight(String, String, Task)}
+ * and {@link HighlightTuple}.
+ */
+ protected void setHighlight(String name, HighlightTuple tuple) {
+ highlights.put(name, tuple);
+ }
+
+ /** As {@link #setHighlight(String, HighlightTuple)}, convenience for recording an item which is intended to be ongoing. */
+ protected void highlightOngoing(String name, String description) {
+ highlights.put(name, new HighlightTuple(description, 0, null));
--- End diff --
Should be calling setHighlight, rather than putting directly into the map. This would allow these methods to remain uncoupled from how we store the highlights. Same for the below highlight methods.
---
[GitHub] brooklyn-server pull request #818: More on adjunct highlights
Posted by Graeme-Miller <gi...@git.apache.org>.
Github user Graeme-Miller commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/818#discussion_r138642145
--- Diff: core/src/main/java/org/apache/brooklyn/core/objs/AbstractEntityAdjunct.java ---
@@ -568,6 +570,98 @@ public void setUniqueTag(String uniqueTag) {
return highlightsToReturn;
}
+ /** Records a named highlight against this object, for persistence and API access.
+ * See common highlights including {@link #HIGHLIGHT_NAME_LAST_ACTION} and
+ * {@link #HIGHLIGHT_NAME_LAST_CONFIRMATION}.
+ * Also see convenience methods eg {@link #highlightOngoing(String, String)} and {@link #highlight(String, String, Task)}
+ * and {@link HighlightTuple}.
+ */
+ protected void setHighlight(String name, HighlightTuple tuple) {
+ highlights.put(name, tuple);
+ }
+
+ /** As {@link #setHighlight(String, HighlightTuple)}, convenience for recording an item which is intended to be ongoing. */
+ protected void highlightOngoing(String name, String description) {
+ highlights.put(name, new HighlightTuple(description, 0, null));
+ }
+ /** As {@link #setHighlight(String, HighlightTuple)}, convenience for recording an item with the current time. */
+ protected void highlightNow(String name, String description) {
+ highlights.put(name, new HighlightTuple(description, System.currentTimeMillis(), null));
+ }
+ /** As {@link #setHighlight(String, HighlightTuple)}, convenience for recording an item with the current time and given task. */
+ protected void highlight(String name, String description, @Nullable Task<?> t) {
+ highlights.put(name, new HighlightTuple(description, System.currentTimeMillis(), t!=null ? t.getId() : null));
+ }
+
+ /** As {@link #setHighlight(String, HighlightTuple)} for {@link #HIGHLIGHT_NAME_TRIGGERS} (as ongoing). */
+ protected void highlightTriggers(String description) {
+ highlightOngoing(HIGHLIGHT_NAME_TRIGGERS, description);
+ }
+ protected <T> void highlightTriggers(Sensor<?> s, Object source) {
+ highlightTriggers(Collections.singleton(s), Collections.singleton(source));
+ }
+ protected <T> void highlightTriggers(Iterable<? extends Sensor<? extends T>> s, Object source) {
+ highlightTriggers(s, (Iterable<?>) (source instanceof Iterable ? (Iterable<?>)source : Collections.singleton(source)));
+ }
+ protected <U> void highlightTriggers(Sensor<?> s, Iterable<U> sources) {
+ highlightTriggers(Collections.singleton(s), sources);
+ }
+ protected <T,U> void highlightTriggers(Iterable<? extends Sensor<? extends T>> sensors, Iterable<U> sources) {
--- End diff --
This method would benefit from comments to explain what it is doing, with multiple loops and nested ifs it isn't easy to tell at a glance.
---
[GitHub] brooklyn-server issue #818: More on adjunct highlights
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the issue:
https://github.com/apache/brooklyn-server/pull/818
See https://github.com/apache/brooklyn-server/pull/818/commits/5ae9c41c1dc8d2884598a2375581d2c4dc14ce14#diff-0724222c7ef44a5337e1e8f44d9d5faeR114 for illustration of what I mean by an easier adjunct java author API (just some convenience methods to allow DRY).
---
[GitHub] brooklyn-server issue #818: More on adjunct highlights
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the issue:
https://github.com/apache/brooklyn-server/pull/818
fixed test failure as streaming with lambdas enforces generic information (did not know that - kinda cool actually)
---
[GitHub] brooklyn-server pull request #818: More on adjunct highlights
Posted by Graeme-Miller <gi...@git.apache.org>.
Github user Graeme-Miller commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/818#discussion_r138651745
--- Diff: core/src/main/java/org/apache/brooklyn/core/objs/AbstractEntityAdjunct.java ---
@@ -568,6 +570,98 @@ public void setUniqueTag(String uniqueTag) {
return highlightsToReturn;
}
+ /** Records a named highlight against this object, for persistence and API access.
+ * See common highlights including {@link #HIGHLIGHT_NAME_LAST_ACTION} and
+ * {@link #HIGHLIGHT_NAME_LAST_CONFIRMATION}.
+ * Also see convenience methods eg {@link #highlightOngoing(String, String)} and {@link #highlight(String, String, Task)}
+ * and {@link HighlightTuple}.
+ */
+ protected void setHighlight(String name, HighlightTuple tuple) {
+ highlights.put(name, tuple);
+ }
+
+ /** As {@link #setHighlight(String, HighlightTuple)}, convenience for recording an item which is intended to be ongoing. */
+ protected void highlightOngoing(String name, String description) {
+ highlights.put(name, new HighlightTuple(description, 0, null));
+ }
+ /** As {@link #setHighlight(String, HighlightTuple)}, convenience for recording an item with the current time. */
+ protected void highlightNow(String name, String description) {
+ highlights.put(name, new HighlightTuple(description, System.currentTimeMillis(), null));
+ }
+ /** As {@link #setHighlight(String, HighlightTuple)}, convenience for recording an item with the current time and given task. */
+ protected void highlight(String name, String description, @Nullable Task<?> t) {
+ highlights.put(name, new HighlightTuple(description, System.currentTimeMillis(), t!=null ? t.getId() : null));
+ }
+
+ /** As {@link #setHighlight(String, HighlightTuple)} for {@link #HIGHLIGHT_NAME_TRIGGERS} (as ongoing). */
+ protected void highlightTriggers(String description) {
+ highlightOngoing(HIGHLIGHT_NAME_TRIGGERS, description);
+ }
+ protected <T> void highlightTriggers(Sensor<?> s, Object source) {
+ highlightTriggers(Collections.singleton(s), Collections.singleton(source));
+ }
+ protected <T> void highlightTriggers(Iterable<? extends Sensor<? extends T>> s, Object source) {
+ highlightTriggers(s, (Iterable<?>) (source instanceof Iterable ? (Iterable<?>)source : Collections.singleton(source)));
+ }
+ protected <U> void highlightTriggers(Sensor<?> s, Iterable<U> sources) {
+ highlightTriggers(Collections.singleton(s), sources);
+ }
+ protected <T,U> void highlightTriggers(Iterable<? extends Sensor<? extends T>> sensors, Iterable<U> sources) {
+ StringBuilder msg = new StringBuilder("Listening for ");
+ boolean firstWord = true;
+ if (sensors!=null) for (Object s: sensors) {
+ if (s==null) continue;
+ if (!firstWord) { msg.append(", "); } else { firstWord = false; }
+ // s is normally a sensor but forgive other things if caller cheated generics
+ msg.append(s instanceof Sensor ? ((Sensor<?>)s).getName() : s.toString());
+ }
+ if (firstWord) msg.append("<nothing>");
+
+ firstWord = true;
--- End diff --
lines 620 -> 639 are confusing and it is difficult to extract what the code is supposed to do. I had a go at what I think is a simpler solution:
```
if(!myList.isEmpty() && !(myList.size() == 1 && myList.iterator().next().equals(getEntity())) {
msg.append(" on ");
List<String> stringList = myList.stream().map((Sensor s) -> {
if (s.equals(getEntity())) {
return "self";
} else if (s instanceof Sensor) {
return ((Sensor<?>) s).getName();
} else {
return s.toString();
}
}).collect(Collectors.toList());
msg.append(String.join(",", stringList));
}
```
---
[GitHub] brooklyn-server pull request #818: More on adjunct highlights
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/brooklyn-server/pull/818
---
[GitHub] brooklyn-server pull request #818: More on adjunct highlights
Posted by Graeme-Miller <gi...@git.apache.org>.
Github user Graeme-Miller commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/818#discussion_r138642446
--- Diff: core/src/main/java/org/apache/brooklyn/core/objs/AbstractEntityAdjunct.java ---
@@ -568,6 +570,98 @@ public void setUniqueTag(String uniqueTag) {
return highlightsToReturn;
}
+ /** Records a named highlight against this object, for persistence and API access.
+ * See common highlights including {@link #HIGHLIGHT_NAME_LAST_ACTION} and
+ * {@link #HIGHLIGHT_NAME_LAST_CONFIRMATION}.
+ * Also see convenience methods eg {@link #highlightOngoing(String, String)} and {@link #highlight(String, String, Task)}
+ * and {@link HighlightTuple}.
+ */
+ protected void setHighlight(String name, HighlightTuple tuple) {
+ highlights.put(name, tuple);
+ }
+
+ /** As {@link #setHighlight(String, HighlightTuple)}, convenience for recording an item which is intended to be ongoing. */
+ protected void highlightOngoing(String name, String description) {
+ highlights.put(name, new HighlightTuple(description, 0, null));
+ }
+ /** As {@link #setHighlight(String, HighlightTuple)}, convenience for recording an item with the current time. */
+ protected void highlightNow(String name, String description) {
+ highlights.put(name, new HighlightTuple(description, System.currentTimeMillis(), null));
+ }
+ /** As {@link #setHighlight(String, HighlightTuple)}, convenience for recording an item with the current time and given task. */
+ protected void highlight(String name, String description, @Nullable Task<?> t) {
+ highlights.put(name, new HighlightTuple(description, System.currentTimeMillis(), t!=null ? t.getId() : null));
+ }
+
+ /** As {@link #setHighlight(String, HighlightTuple)} for {@link #HIGHLIGHT_NAME_TRIGGERS} (as ongoing). */
+ protected void highlightTriggers(String description) {
+ highlightOngoing(HIGHLIGHT_NAME_TRIGGERS, description);
+ }
+ protected <T> void highlightTriggers(Sensor<?> s, Object source) {
+ highlightTriggers(Collections.singleton(s), Collections.singleton(source));
+ }
+ protected <T> void highlightTriggers(Iterable<? extends Sensor<? extends T>> s, Object source) {
+ highlightTriggers(s, (Iterable<?>) (source instanceof Iterable ? (Iterable<?>)source : Collections.singleton(source)));
+ }
+ protected <U> void highlightTriggers(Sensor<?> s, Iterable<U> sources) {
+ highlightTriggers(Collections.singleton(s), sources);
+ }
+ protected <T,U> void highlightTriggers(Iterable<? extends Sensor<? extends T>> sensors, Iterable<U> sources) {
+ StringBuilder msg = new StringBuilder("Listening for ");
+ boolean firstWord = true;
+ if (sensors!=null) for (Object s: sensors) {
--- End diff --
do we have a style guide for brooklyn? I have never seen someone put a for loop on the same line as an if. Seems to break with our coding conventions.
---
[GitHub] brooklyn-server pull request #818: More on adjunct highlights
Posted by grkvlt <gi...@git.apache.org>.
Github user grkvlt commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/818#discussion_r138690532
--- Diff: core/src/main/java/org/apache/brooklyn/core/objs/AbstractEntityAdjunct.java ---
@@ -568,6 +570,98 @@ public void setUniqueTag(String uniqueTag) {
return highlightsToReturn;
}
+ /** Records a named highlight against this object, for persistence and API access.
+ * See common highlights including {@link #HIGHLIGHT_NAME_LAST_ACTION} and
+ * {@link #HIGHLIGHT_NAME_LAST_CONFIRMATION}.
+ * Also see convenience methods eg {@link #highlightOngoing(String, String)} and {@link #highlight(String, String, Task)}
+ * and {@link HighlightTuple}.
+ */
+ protected void setHighlight(String name, HighlightTuple tuple) {
+ highlights.put(name, tuple);
+ }
+
+ /** As {@link #setHighlight(String, HighlightTuple)}, convenience for recording an item which is intended to be ongoing. */
+ protected void highlightOngoing(String name, String description) {
+ highlights.put(name, new HighlightTuple(description, 0, null));
+ }
+ /** As {@link #setHighlight(String, HighlightTuple)}, convenience for recording an item with the current time. */
+ protected void highlightNow(String name, String description) {
+ highlights.put(name, new HighlightTuple(description, System.currentTimeMillis(), null));
+ }
+ /** As {@link #setHighlight(String, HighlightTuple)}, convenience for recording an item with the current time and given task. */
+ protected void highlight(String name, String description, @Nullable Task<?> t) {
+ highlights.put(name, new HighlightTuple(description, System.currentTimeMillis(), t!=null ? t.getId() : null));
+ }
+
+ /** As {@link #setHighlight(String, HighlightTuple)} for {@link #HIGHLIGHT_NAME_TRIGGERS} (as ongoing). */
+ protected void highlightTriggers(String description) {
+ highlightOngoing(HIGHLIGHT_NAME_TRIGGERS, description);
+ }
+ protected <T> void highlightTriggers(Sensor<?> s, Object source) {
+ highlightTriggers(Collections.singleton(s), Collections.singleton(source));
+ }
+ protected <T> void highlightTriggers(Iterable<? extends Sensor<? extends T>> s, Object source) {
+ highlightTriggers(s, (Iterable<?>) (source instanceof Iterable ? (Iterable<?>)source : Collections.singleton(source)));
+ }
+ protected <U> void highlightTriggers(Sensor<?> s, Iterable<U> sources) {
+ highlightTriggers(Collections.singleton(s), sources);
+ }
+ protected <T,U> void highlightTriggers(Iterable<? extends Sensor<? extends T>> sensors, Iterable<U> sources) {
+ StringBuilder msg = new StringBuilder("Listening for ");
+ boolean firstWord = true;
+ if (sensors!=null) for (Object s: sensors) {
+ if (s==null) continue;
+ if (!firstWord) { msg.append(", "); } else { firstWord = false; }
+ // s is normally a sensor but forgive other things if caller cheated generics
+ msg.append(s instanceof Sensor ? ((Sensor<?>)s).getName() : s.toString());
+ }
+ if (firstWord) msg.append("<nothing>");
+
+ firstWord = true;
--- End diff --
Interesting @Graeme-Miller - Not sure that you need the second clause in your `if`, as the body handles the `self` output the same as the original code. I'd prefer the `Collectors.joining(", ")` collector, to get a `String` out directly... Also, I'm not sure you should try and chest the lambda type inference like that, just go `s -> { /* stuff */ }` and it'll be fine. Redoing the whole method with lambdas gave me this, which is almost exactly the same length ;)
```
protected <T,U> void highlightTriggers(Iterable<? extends Sensor<? extends T>> sensors, Iterable<U> sources) {
StringBuilder msg = new StringBuilder("Listening for ");
if (Iterables.isEmpty(sensors)) {
msg.append("<nothing>");
} else {
String sensorsText = StreamSupport.stream(sensors.spliterator(), false)
.filter(s -> s != null)
.map(s -> {
if (s instanceof Sensor) {
return ((Sensor<?>) s).getName();
} else {
return s.toString();
}
})
.collect(Collectors.joining(", "));
msg.append(sensorsText);
}
if (!Iterables.isEmpty(sources)) {
String sourcesText = StreamSupport.stream(sources.spliterator(), false)
.filter(s -> s != null)
.map(s -> {
if (s.equals(getEntity())) {
return "self";
} else if (s instanceof Sensor) {
return ((Sensor<?>) s).getName();
} else {
return s.toString();
}
})
.collect(Collectors.joining(", "));
msg.append(" on ")
.append(sourcesText);
}
highlightTriggers(msg.toString());
}
```
---
[GitHub] brooklyn-server issue #818: More on adjunct highlights
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the issue:
https://github.com/apache/brooklyn-server/pull/818
all addressed, thanks. if build passes then let's merge.
---
[GitHub] brooklyn-server issue #818: More on adjunct highlights
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the issue:
https://github.com/apache/brooklyn-server/pull/818
Now has highlights on most things.
---