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 2017/11/06 14:40:56 UTC
[GitHub] brooklyn-server pull request #880: Fix rebind EntitySpec with policies ref
GitHub user aledsage opened a pull request:
https://github.com/apache/brooklyn-server/pull/880
Fix rebind EntitySpec with policies ref
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/aledsage/brooklyn-server fix-rebind-EntitySpec-with-policies
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/brooklyn-server/pull/880.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 #880
----
commit c57b85065c3df5ba59f2036c6e105e0e390c1483
Author: Aled Sage <al...@gmail.com>
Date: 2017-11-06T13:11:20Z
Fix rebind EntitySpec with policies ref
----
---
[GitHub] brooklyn-server issue #880: Fix rebind EntitySpec with policies ref
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the issue:
https://github.com/apache/brooklyn-server/pull/880
aha gotcha - do the `s/policy/enricher` if you haven't already then good to merge
---
[GitHub] brooklyn-server pull request #880: Fix rebind EntitySpec with policies ref
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/880#discussion_r149097024
--- Diff: api/src/main/java/org/apache/brooklyn/api/entity/EntitySpec.java ---
@@ -112,12 +118,29 @@
private final List<Entity> members = Lists.newArrayList();
private final List<Group> groups = Lists.newArrayList();
private volatile boolean immutable;
-
+
+ // Kept for backwards compatibility of persisted state
+ private List<Policy> policies;
+ private List<Enricher> enrichers;
+
public EntitySpec(Class<T> type) {
super(type);
}
@Override
+ protected Object readResolve() {
+ if (policies != null && policies.size() > 0) {
--- End diff --
personal preference for `&& !policies.isEmpty()`
---
[GitHub] brooklyn-server issue #880: Fix rebind EntitySpec with policies ref
Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the issue:
https://github.com/apache/brooklyn-server/pull/880
Good thorough fix.
One item needing fixed -- should be `enrichers = null` in second `readResolve` block.
Are there known instances where a policy/enricher would have been persisted and is now being ignored? Or was the rebind error simply happening for empty lists and relatively unimportant items?
---
[GitHub] brooklyn-server issue #880: Fix rebind EntitySpec with policies ref
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-server/pull/880
retest this please
failure is unrelated:
```
java.lang.AssertionError: result=Present[value=Present[value=foo]] expected [true] but found [false]
at org.apache.brooklyn.util.core.task.ValueResolverTest.testUnsubmittedTaskWithExecutionContextTimesOutWhenImmediate(ValueResolverTest.java:159)
```
---
[GitHub] brooklyn-server pull request #880: Fix rebind EntitySpec with policies ref
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/brooklyn-server/pull/880
---
[GitHub] brooklyn-server issue #880: Fix rebind EntitySpec with policies ref
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the issue:
https://github.com/apache/brooklyn-server/pull/880
@ahgittin thanks. The rebind error was for an empty list of policies (which I think is the default, so rebind should have been failing for any upgrade from old to new persisted state!).
I don't see how we could have had a policy/enricher instance being referenced by an EntitySpec in historic persisted state - it would have failed to rebind in older versions of Brooklyn because the dangling-policy-ref error I described in javadoc of the test.
---