You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by aledsage <gi...@git.apache.org> on 2014/11/01 00:57:02 UTC
[GitHub] incubator-brooklyn pull request: Fix ServiceStateLogic integration...
GitHub user aledsage opened a pull request:
https://github.com/apache/incubator-brooklyn/pull/290
Fix ServiceStateLogic integration tests
Fixes `brooklyn.rest.client.ApplicationResourceIntegrationTest`.
- Don’t modify the existing attribute value in `ServiceStateLogic`. Instead take a copy and modify that.
- Also fixes (and tidies) assertions in `brooklyn.rest.client.ApplicationResourceIntegrationTest`
- `BrooklynRestResourceUtils`: don’t propagate to the top-level app the service_up, service_not_up_indicators, etc. These are populated automatic by app.initEnrichers, which look at the child entities.
- Adds `EntityLocal.modifyAttribute(AttributeSensor, Function)` for atomic (sequential) updates to an attribute, where the new value is computed from the old value.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/aledsage/incubator-brooklyn fix/ServiceStateLogic-IntegrationTests
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-brooklyn/pull/290.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 #290
----
commit a27260e87259700106cad1bb293315cb78ac946f
Author: Aled Sage <al...@gmail.com>
Date: 2014-10-31T22:57:14Z
Add EntityLocal.modifyAttribute(AttributeSensor, Function)
- For atomic (sequential) updates to an attribute, where the new
value is computed from the old value.
commit b5685c254636666fae49645f9d18bf27b0918bf2
Author: Aled Sage <al...@gmail.com>
Date: 2014-10-31T23:07:02Z
Fix ServiceStateLogic updating map
- Don’t modify the existing attribute value. Instead take a copy
and modify that.
- Also fixes (and tidies) assertions in
brooklyn.rest.client.ApplicationResourceIntegrationTest
- BrooklynRestResourceUtils: don’t propagate to the top-level app
the service_up, service_not_up_indicators, etc.
These are populated automatic by app.initEnrichers, which looks
at the child entities.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Fix ServiceStateLogic integration...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/290#discussion_r19703524
--- Diff: usage/rest-client/src/test/java/brooklyn/rest/client/ApplicationResourceIntegrationTest.java ---
@@ -88,8 +90,8 @@ public void setUp() throws Exception {
WebAppContext context;
// running in source mode; need to use special classpath
- context = new WebAppContext("src/test/webapp", "/");
- context.setExtraClasspath("./target/test-rest-server/");
+ context = new WebAppContext("/Users/aled/repos/apache-asf/incubator-brooklyn/usage/rest-client/src/test/webapp", "/");
+ context.setExtraClasspath("/Users/aled/repos/apache-asf/incubator-brooklyn/usage/rest-client/target/test-rest-server/");
--- End diff --
revert this I think:)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Fix ServiceStateLogic integration...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/290#discussion_r19703516
--- Diff: api/src/main/java/brooklyn/entity/basic/EntityLocal.java ---
@@ -66,14 +68,27 @@
<T> T setConfig(HasConfigKey<T> key, Task<T> val);
/**
- * Sets the {@link Sensor} data for the given attribute to the specified value.
+ * Sets the {@link AttributeSensor} data for the given attribute to the specified value.
*
* This can be used to "enrich" the entity, such as adding aggregated information,
* rolling averages, etc.
*
* @return the old value for the attribute (possibly {@code null})
*/
- <T> T setAttribute(AttributeSensor<T> sensor, T val);
+ <T> T setAttribute(AttributeSensor<T> attribute, T val);
+
+ /**
+ * Atomically modifies the {@link AttributeSensor}, ensuring that only one modification is done
+ * at a time.
--- End diff --
Worth saying implementations typically synchronize on the backing AttributeMap?
Or probably better, say: "For details of the synchronization model used to achieve this, refer to the underlying attribute store (e.g. AttributeMap)."
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Fix ServiceStateLogic integration...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/290#discussion_r19703502
--- Diff: core/src/main/java/brooklyn/entity/basic/ServiceStateLogic.java ---
@@ -424,11 +444,15 @@ protected void onUpdated() {
}
// override superclass to publish multiple sensors
- if (getConfig(DERIVE_SERVICE_PROBLEMS))
- updateMapSensor(SERVICE_PROBLEMS, computeServiceProblems());
+ if (getConfig(DERIVE_SERVICE_PROBLEMS)) {
+ Object newval = computeServiceProblems();
+ updateMapSensor(SERVICE_PROBLEMS, newval);
+ }
--- End diff --
personal preference, but to me this makes it harder to read
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Fix ServiceStateLogic integration...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/290#discussion_r19703521
--- Diff: core/src/main/java/brooklyn/event/basic/AttributeMap.java ---
@@ -132,6 +134,21 @@ private void checkPath(Collection<String> path) {
return (isNull(oldValue)) ? null : oldValue;
}
+ public <T> T modify(AttributeSensor<T> attribute, Function<? super T, Maybe<T>> modifier) {
--- End diff --
And say in javadoc on the class and/or this impl that where atomicity is desired, the methods in this class synchronize on the map.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Fix ServiceStateLogic integration...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-brooklyn/pull/290
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Fix ServiceStateLogic integration...
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/290#issuecomment-61364385
changes look good. i like the `modify` operation.
just a few minor but necessary comments, then good to merge.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Fix ServiceStateLogic integration...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/290#issuecomment-61470806
Test failure is unrelated. I'll look at that in a separate PR.
Merging now.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-brooklyn pull request: Fix ServiceStateLogic integration...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/290#issuecomment-61458070
Thanks @ahgittin - I have incorporated comments and rebased - will wait for asfbot build, and then merge.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---