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/07/01 11:29:17 UTC

[GitHub] incubator-brooklyn pull request: BROOKLYN-14: rebind policies in s...

GitHub user aledsage opened a pull request:

    https://github.com/apache/incubator-brooklyn/pull/35

    BROOKLYN-14: rebind policies in separate phase

    - Only add policies+enrichers to entities after all entities have had
      their state set, and all relationships (parent-child, 
      group membership, locations) have been set up
    - Also corrects spelling of “doReconsruct” in BasicPolicyRebindSupport
      etc
    - Improves comments in RebindManagerImpl on rebind’s phases

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

    $ git pull https://github.com/aledsage/incubator-brooklyn fix/rebind-addPolicyInSeparatePhase

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

    https://github.com/apache/incubator-brooklyn/pull/35.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 #35
    
----
commit 59ce9e8b04b4d226436edab96c9e8b4ec2539154
Author: Aled Sage <al...@gmail.com>
Date:   2014-07-01T09:28:33Z

    BROOKLYN-14: rebind policies in separate phase
    
    - Only add policies+enrichers to entities after all entities have had
      their state set, and all relationships (parent-child, 
      group membership, locations) have been set up
    - Also corrects spelling of “doReconsruct” in BasicPolicyRebindSupport
      etc
    - Improves comments in RebindManagerImpl on rebind’s phases

----


---
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: BROOKLYN-14: rebind policies in s...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/incubator-brooklyn/pull/35


---
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: BROOKLYN-14: rebind policies in s...

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/35#discussion_r14503156
  
    --- Diff: core/src/main/java/brooklyn/entity/rebind/RebindManagerImpl.java ---
    @@ -385,7 +396,35 @@ public ChangeListener getChangeListener() {
                         }
                     }
                 }
    +
    +            //
    +            // PHASE THREE
    +            //
    +            
    +            // Associate policies+enrichers with entities
    +            LOG.debug("RebindManager reconstructing entities");
    +            for (EntityMemento entityMemento : sortParentFirst(memento.getEntityMementos()).values()) {
    +                Entity entity = rebindContext.getEntity(entityMemento.getId());
    +                if (LOG.isDebugEnabled()) LOG.debug("RebindManager reconstructing entity {}", entityMemento);
    +    
    +                if (entity == null) {
    +                    // usually because of creation-failure, when not using fail-fast
    +                    exceptionHandler.onEntityNotFound(entityMemento.getId());
    +                } else {
    +                    try {
    +                        entityMemento.injectTypeClass(entity.getClass());
    --- End diff --
    
    this call sticks out a little bit, but i can imagine why it's needed, we're doing more with the memento now whereas previously we just threw it away


---
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: BROOKLYN-14: rebind policies in s...

Posted by ahgittin <gi...@git.apache.org>.
Github user ahgittin commented on the pull request:

    https://github.com/apache/incubator-brooklyn/pull/35#issuecomment-47886513
  
    looks good, works for me.  i have resolved conflicts w #34 and merged.


---
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: BROOKLYN-14: rebind policies in s...

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/35#discussion_r14502877
  
    --- Diff: core/src/main/java/brooklyn/entity/rebind/BasicEntityRebindSupport.java ---
    @@ -98,6 +96,32 @@ public void reconstruct(RebindContext rebindContext, EntityMemento memento) {
             ((AbstractEntity)entity).rebind();
         }
         
    +    @Override
    +    public void addPolicies(RebindContext rebindContext, EntityMemento memento) {
    +        for (String policyId : memento.getPolicies()) {
    +            AbstractPolicy policy = (AbstractPolicy) rebindContext.getPolicy(policyId);
    +            if (policy != null) {
    +                entity.addPolicy(policy);
    +            } else {
    +                LOG.warn("Policy not found; discarding policy {} of entity {}({})",
    +                        new Object[] {policyId, memento.getType(), memento.getId()});
    +            }
    +        }
    +    }
    +    
    +    @Override
    +    public void addEnrichers(RebindContext rebindContext, EntityMemento memento) {
    +        for (String enricherId : memento.getEnrichers()) {
    +            AbstractEnricher enricher = (AbstractEnricher) rebindContext.getEnricher(enricherId);
    +            if (enricher != null) {
    +                entity.addEnricher(enricher);
    --- End diff --
    
    as with `addPolicies`, after #34 need to `rebindContext.getExceptionHandler().onAddEnricherFailed(entity, enricher, e);`


---
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: BROOKLYN-14: rebind policies in s...

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/35#discussion_r14502865
  
    --- Diff: core/src/main/java/brooklyn/entity/rebind/BasicEntityRebindSupport.java ---
    @@ -98,6 +96,32 @@ public void reconstruct(RebindContext rebindContext, EntityMemento memento) {
             ((AbstractEntity)entity).rebind();
         }
         
    +    @Override
    +    public void addPolicies(RebindContext rebindContext, EntityMemento memento) {
    +        for (String policyId : memento.getPolicies()) {
    +            AbstractPolicy policy = (AbstractPolicy) rebindContext.getPolicy(policyId);
    +            if (policy != null) {
    +                entity.addPolicy(policy);
    --- End diff --
    
    conflicts w #34 which catch `Exception e` and do `rebindContext.getExceptionHandler().onAddPolicyFailed(entity, policy, e);`


---
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.
---