You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by GitBox <gi...@apache.org> on 2022/06/08 16:57:07 UTC
[GitHub] [brooklyn-server] ahgittin opened a new pull request, #1320: Introduce entity adjunct proxy, fix deserialization edge case
ahgittin opened a new pull request, #1320:
URL: https://github.com/apache/brooklyn-server/pull/1320
If one BrooklynObject refers to another in an unusual place (eg in a map, not simply policies on entities or vice versa) and the referrer is deserialized before the referent, Apache Brooklyn would give an error. With this PR, in these cases we introduce an EntityAdjunctProxy, similar to the EntityProxy (long since used for the much more common case of entities referring to other entities).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-server] ahgittin commented on a diff in pull request #1320: Introduce entity adjunct proxy, fix deserialization edge case
Posted by GitBox <gi...@apache.org>.
ahgittin commented on code in PR #1320:
URL: https://github.com/apache/brooklyn-server/pull/1320#discussion_r902510318
##########
core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/RebindIteration.java:
##########
@@ -484,11 +483,23 @@ protected void instantiateLocationsAndEntities() {
}
}
+ protected Map<String,EntityAdjunct> adjunctProxies = MutableMap.of();
+ protected <T extends EntityAdjunct> T createAdjunctProxy(Class<T> adjunctType, String id) {
+ return (T) adjunctProxies.computeIfAbsent(id, (id2) -> EntityAdjuncts.createProxy(adjunctType, null) );
+ }
+
protected void instantiateMementos() throws IOException {
checkEnteringPhase(4);
+ if (!adjunctProxies.isEmpty()) {
+ LOG.warn("Had stale adjunct information when rebinding; ignoring: "+adjunctProxies);
+ }
+ adjunctProxies.clear();
+
Review Comment:
it's possible but we might decide to remove the logging at some point so i prefer to always clear
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-server] jcabrerizo merged pull request #1320: Introduce entity adjunct proxy, fix deserialization edge case
Posted by GitBox <gi...@apache.org>.
jcabrerizo merged PR #1320:
URL: https://github.com/apache/brooklyn-server/pull/1320
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-server] jcabrerizo commented on pull request #1320: Introduce entity adjunct proxy, fix deserialization edge case
Posted by GitBox <gi...@apache.org>.
jcabrerizo commented on PR #1320:
URL: https://github.com/apache/brooklyn-server/pull/1320#issuecomment-1163250914
Thanks for this @ahgittin
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-server] jcabrerizo commented on a diff in pull request #1320: Introduce entity adjunct proxy, fix deserialization edge case
Posted by GitBox <gi...@apache.org>.
jcabrerizo commented on code in PR #1320:
URL: https://github.com/apache/brooklyn-server/pull/1320#discussion_r895612711
##########
core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/RebindIteration.java:
##########
@@ -484,11 +483,23 @@ protected void instantiateLocationsAndEntities() {
}
}
+ protected Map<String,EntityAdjunct> adjunctProxies = MutableMap.of();
+ protected <T extends EntityAdjunct> T createAdjunctProxy(Class<T> adjunctType, String id) {
+ return (T) adjunctProxies.computeIfAbsent(id, (id2) -> EntityAdjuncts.createProxy(adjunctType, null) );
+ }
+
protected void instantiateMementos() throws IOException {
checkEnteringPhase(4);
+ if (!adjunctProxies.isEmpty()) {
+ LOG.warn("Had stale adjunct information when rebinding; ignoring: "+adjunctProxies);
+ }
+ adjunctProxies.clear();
+
Review Comment:
Clear is only need if is not empty, moving this inside the exiting `if` above will be a minor improve
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [brooklyn-server] ahgittin commented on pull request #1320: Introduce entity adjunct proxy, fix deserialization edge case
Posted by GitBox <gi...@apache.org>.
ahgittin commented on PR #1320:
URL: https://github.com/apache/brooklyn-server/pull/1320#issuecomment-1161630263
Two new commits to handle case where adjunct is in a set and so needs access to hashcode and related methods. Now uses the ID to compute hashcode, to align with entity and ensure hashcode doesn't change when proxy gets assigned the delegate. (As hashcodes were not persisted and had no guarantee about the value there should be no issue with changing the adjunct hashcode behaviour.)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@brooklyn.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org