Return-Path: X-Original-To: archive-asf-public-internal@cust-asf2.ponee.io Delivered-To: archive-asf-public-internal@cust-asf2.ponee.io Received: from cust-asf.ponee.io (cust-asf.ponee.io [163.172.22.183]) by cust-asf2.ponee.io (Postfix) with ESMTP id B07DE200D22 for ; Fri, 6 Oct 2017 10:06:29 +0200 (CEST) Received: by cust-asf.ponee.io (Postfix) id AF41B1609DF; Fri, 6 Oct 2017 08:06:29 +0000 (UTC) Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by cust-asf.ponee.io (Postfix) with SMTP id 8C07F1609D0 for ; Fri, 6 Oct 2017 10:06:28 +0200 (CEST) Received: (qmail 46193 invoked by uid 500); 6 Oct 2017 08:06:27 -0000 Mailing-List: contact commits-help@brooklyn.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@brooklyn.apache.org Delivered-To: mailing list commits@brooklyn.apache.org Received: (qmail 45633 invoked by uid 99); 6 Oct 2017 08:06:26 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 06 Oct 2017 08:06:26 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 7727DF5CF4; Fri, 6 Oct 2017 08:06:25 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: heneveld@apache.org To: commits@brooklyn.apache.org Date: Fri, 06 Oct 2017 08:06:46 -0000 Message-Id: <5da168e076574a16a9d287404efb7d0a@git.apache.org> In-Reply-To: References: X-Mailer: ASF-Git Admin Mailer Subject: [22/23] brooklyn-server git commit: address review comments archived-at: Fri, 06 Oct 2017 08:06:29 -0000 address review comments Project: http://git-wip-us.apache.org/repos/asf/brooklyn-server/repo Commit: http://git-wip-us.apache.org/repos/asf/brooklyn-server/commit/2dcb0a03 Tree: http://git-wip-us.apache.org/repos/asf/brooklyn-server/tree/2dcb0a03 Diff: http://git-wip-us.apache.org/repos/asf/brooklyn-server/diff/2dcb0a03 Branch: refs/heads/master Commit: 2dcb0a03a9e9b27a81535a13bd48d1958f8984d9 Parents: 9213f0e Author: Alex Heneveld Authored: Fri Oct 6 01:04:34 2017 +0100 Committer: Alex Heneveld Committed: Fri Oct 6 01:12:43 2017 +0100 ---------------------------------------------------------------------- .../apache/brooklyn/api/mgmt/ExecutionContext.java | 2 +- .../apache/brooklyn/api/mgmt/ManagementContext.java | 3 +++ .../brooklyn/core/config/ConfigConstraints.java | 2 +- .../mgmt/internal/AbstractManagementContext.java | 16 ++++++++++++++++ .../internal/NonDeploymentManagementContext.java | 9 +++++++++ .../core/mgmt/rebind/RebindManagerImpl.java | 3 ++- .../brooklyn/core/objs/AbstractEntityAdjunct.java | 6 +----- .../util/core/task/BasicExecutionManager.java | 5 ++++- .../brooklyn/core/entity/EntityAssertsTest.java | 3 ++- .../mgmt/internal/LocalSubscriptionManagerTest.java | 8 +++----- 10 files changed, 42 insertions(+), 15 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/2dcb0a03/api/src/main/java/org/apache/brooklyn/api/mgmt/ExecutionContext.java ---------------------------------------------------------------------- diff --git a/api/src/main/java/org/apache/brooklyn/api/mgmt/ExecutionContext.java b/api/src/main/java/org/apache/brooklyn/api/mgmt/ExecutionContext.java index 2d8fe23..d303ba9 100644 --- a/api/src/main/java/org/apache/brooklyn/api/mgmt/ExecutionContext.java +++ b/api/src/main/java/org/apache/brooklyn/api/mgmt/ExecutionContext.java @@ -62,7 +62,7 @@ public interface ExecutionContext extends Executor { @Deprecated Task submit(Callable callable); - /** {@link ExecutionManager#submit(String Runnable) */ + /** {@link ExecutionManager#submit(String, Runnable) */ Task submit(String displayName, Runnable runnable); /** {@link ExecutionManager#submit(String, Callable) */ http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/2dcb0a03/api/src/main/java/org/apache/brooklyn/api/mgmt/ManagementContext.java ---------------------------------------------------------------------- diff --git a/api/src/main/java/org/apache/brooklyn/api/mgmt/ManagementContext.java b/api/src/main/java/org/apache/brooklyn/api/mgmt/ManagementContext.java index 00dd4d3..c1a2c21 100644 --- a/api/src/main/java/org/apache/brooklyn/api/mgmt/ManagementContext.java +++ b/api/src/main/java/org/apache/brooklyn/api/mgmt/ManagementContext.java @@ -166,6 +166,9 @@ public interface ManagementContext { */ ExecutionContext getExecutionContext(Entity entity); + /** As {@link #getExecutionContext(Entity)} where there is also an adjunct */ + ExecutionContext getExecutionContext(Entity e, EntityAdjunct a); + /** * Returns a {@link SubscriptionContext} instance representing subscriptions * (from the {@link SubscriptionManager}) associated with this entity, and capable http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/2dcb0a03/core/src/main/java/org/apache/brooklyn/core/config/ConfigConstraints.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/config/ConfigConstraints.java b/core/src/main/java/org/apache/brooklyn/core/config/ConfigConstraints.java index c4ce81d..682dedb 100644 --- a/core/src/main/java/org/apache/brooklyn/core/config/ConfigConstraints.java +++ b/core/src/main/java/org/apache/brooklyn/core/config/ConfigConstraints.java @@ -117,7 +117,7 @@ public abstract class ConfigConstraints { public Iterable> getViolations() { ExecutionContext exec = getBrooklynObject() instanceof EntityInternal ? ((EntityInternal)getBrooklynObject()).getExecutionContext() : - // getBrooklynObject() instanceof AbstractEntityAdjunct ? ((AbstractEntityAdjunct)getBrooklynObject()).getExecutionContext() : + getBrooklynObject() instanceof AbstractEntityAdjunct ? ((AbstractEntityAdjunct)getBrooklynObject()).getExecutionContext() : null; if (exec!=null) { return exec.get( http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/2dcb0a03/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/AbstractManagementContext.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/AbstractManagementContext.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/AbstractManagementContext.java index 7a0fabd..27645bc 100644 --- a/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/AbstractManagementContext.java +++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/AbstractManagementContext.java @@ -71,6 +71,7 @@ import org.apache.brooklyn.core.mgmt.classloading.JavaBrooklynClassLoadingContex import org.apache.brooklyn.core.mgmt.entitlement.Entitlements; import org.apache.brooklyn.core.mgmt.ha.HighAvailabilityManagerImpl; import org.apache.brooklyn.core.mgmt.rebind.RebindManagerImpl; +import org.apache.brooklyn.core.objs.AbstractEntityAdjunct; import org.apache.brooklyn.core.typereg.BasicBrooklynTypeRegistry; import org.apache.brooklyn.util.collections.MutableList; import org.apache.brooklyn.util.core.ResourceUtils; @@ -245,6 +246,21 @@ public abstract class AbstractManagementContext implements ManagementContextInte return ((EntityInternal)e).getExecutionContext(); } } + + @Override + public ExecutionContext getExecutionContext(Entity e, EntityAdjunct adjunct) { + // BEC is a thin wrapper around EM so fine to create a new one here; but make sure it gets the real entity + if (e instanceof AbstractEntityAdjunct) { + ImmutableSet tags = ImmutableSet.of( + BrooklynTaskTags.tagForContextAdjunct(adjunct), + BrooklynTaskTags.tagForContextEntity(e), + this + ); + return new BasicExecutionContext(getExecutionManager(), tags); + } else { + return ((EntityInternal)e).getExecutionContext(); + } + } @Override public ExecutionContext getServerExecutionContext() { http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/2dcb0a03/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/NonDeploymentManagementContext.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/NonDeploymentManagementContext.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/NonDeploymentManagementContext.java index bac7aeb..b5fd0b5 100644 --- a/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/NonDeploymentManagementContext.java +++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/internal/NonDeploymentManagementContext.java @@ -282,6 +282,15 @@ public class NonDeploymentManagementContext implements ManagementContextInternal } @Override + public ExecutionContext getExecutionContext(Entity entity, EntityAdjunct adjunct) { + if (!this.entity.equals(entity)) throw new IllegalStateException("Non-deployment context "+this+" can only use a single Entity: has "+this.entity+", but passed "+entity); + if (mode==NonDeploymentManagementContextMode.MANAGEMENT_STOPPED) + throw new IllegalStateException("Entity "+entity+" is no longer managed; execution context not available"); + checkInitialManagementContextReal(); + return initialManagementContext.getExecutionContext(entity, adjunct); + } + + @Override public ExecutionContext getServerExecutionContext() { return initialManagementContext.getServerExecutionContext(); } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/2dcb0a03/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/RebindManagerImpl.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/RebindManagerImpl.java b/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/RebindManagerImpl.java index f5a1c0c..0830900 100644 --- a/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/RebindManagerImpl.java +++ b/core/src/main/java/org/apache/brooklyn/core/mgmt/rebind/RebindManagerImpl.java @@ -490,7 +490,8 @@ public class RebindManagerImpl implements RebindManager { ExecutionContext ec = BasicExecutionContext.getCurrentExecutionContext(); if (ec == null) { ec = managementContext.getServerExecutionContext(); - return ec.get(Tasks.>builder().displayName("rebind").dynamic(false).body(() -> rebindImpl(classLoader, exceptionHandler, mode)).build()); + return ec.get(Tasks.>builder().displayName("rebind").dynamic(false) + .body(() -> rebindImpl(classLoader, exceptionHandler, mode)).build()); } else { return rebindImpl(classLoader, exceptionHandler, mode); } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/2dcb0a03/core/src/main/java/org/apache/brooklyn/core/objs/AbstractEntityAdjunct.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/core/objs/AbstractEntityAdjunct.java b/core/src/main/java/org/apache/brooklyn/core/objs/AbstractEntityAdjunct.java index c6cb74c..7b2f6ad 100644 --- a/core/src/main/java/org/apache/brooklyn/core/objs/AbstractEntityAdjunct.java +++ b/core/src/main/java/org/apache/brooklyn/core/objs/AbstractEntityAdjunct.java @@ -54,15 +54,12 @@ import org.apache.brooklyn.core.enricher.AbstractEnricher; import org.apache.brooklyn.core.entity.Entities; import org.apache.brooklyn.core.entity.EntityInternal; import org.apache.brooklyn.core.entity.internal.ConfigUtilsInternal; -import org.apache.brooklyn.core.mgmt.BrooklynTaskTags; import org.apache.brooklyn.core.mgmt.internal.SubscriptionTracker; -import org.apache.brooklyn.util.collections.MutableList; import org.apache.brooklyn.util.collections.MutableSet; import org.apache.brooklyn.util.core.config.ConfigBag; import org.apache.brooklyn.util.core.flags.FlagUtils; import org.apache.brooklyn.util.core.flags.SetFromFlag; import org.apache.brooklyn.util.core.flags.TypeCoercions; -import org.apache.brooklyn.util.core.task.BasicExecutionContext; import org.apache.brooklyn.util.text.Strings; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -428,8 +425,7 @@ public abstract class AbstractEntityAdjunct extends AbstractBrooklynObject imple public void setEntity(EntityLocal entity) { if (destroyed.get()) throw new IllegalStateException("Cannot set entity on a destroyed entity adjunct"); this.entity = entity; - this.execution = new BasicExecutionContext( getManagementContext().getExecutionManager(), - MutableList.of(BrooklynTaskTags.tagForContextAdjunct(this), BrooklynTaskTags.tagForContextEntity(entity)) ); + this.execution = getManagementContext().getExecutionContext(entity, this); if (entity!=null && getCatalogItemId() == null) { setCatalogItemIdAndSearchPath(entity.getCatalogItemId(), entity.getCatalogItemIdSearchPath()); } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/2dcb0a03/core/src/main/java/org/apache/brooklyn/util/core/task/BasicExecutionManager.java ---------------------------------------------------------------------- diff --git a/core/src/main/java/org/apache/brooklyn/util/core/task/BasicExecutionManager.java b/core/src/main/java/org/apache/brooklyn/util/core/task/BasicExecutionManager.java index c7a9855..c53277d 100644 --- a/core/src/main/java/org/apache/brooklyn/util/core/task/BasicExecutionManager.java +++ b/core/src/main/java/org/apache/brooklyn/util/core/task/BasicExecutionManager.java @@ -799,7 +799,10 @@ public class BasicExecutionManager implements ExecutionManager { Task t = Tasks.builder().dynamic(false).displayName(displayName+" (placeholder for "+id+")") .description("Details of the original task have been forgotten.") .body(Callables.returning((T)null)).build(); - ((BasicTask)t).ignoreIfNotRun(); + // don't really want anyone executing the "gone" task... + // also if we are GC'ing tasks then cancelled may help with cleanup + // of sub-tasks that have lost their submitted-by-task reference ? + // also don't want warnings when it's finalized, this means we don't need ignoreIfNotRun() ((BasicTask)t).cancelled = true; return t; } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/2dcb0a03/core/src/test/java/org/apache/brooklyn/core/entity/EntityAssertsTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/brooklyn/core/entity/EntityAssertsTest.java b/core/src/test/java/org/apache/brooklyn/core/entity/EntityAssertsTest.java index 3160518..2967f22 100644 --- a/core/src/test/java/org/apache/brooklyn/core/entity/EntityAssertsTest.java +++ b/core/src/test/java/org/apache/brooklyn/core/entity/EntityAssertsTest.java @@ -83,7 +83,8 @@ public class EntityAssertsTest extends BrooklynAppUnitTestSupport { entity.sensors().set(TestEntity.NAME, "before"); final String after = "after"; - Task assertValue = entity.getExecutionContext().submit("assert attr equals", () -> EntityAsserts.assertAttributeEqualsEventually(entity, TestEntity.NAME, after)); + Task assertValue = entity.getExecutionContext().submit("assert attr equals", + () -> EntityAsserts.assertAttributeEqualsEventually(entity, TestEntity.NAME, after)); entity.sensors().set(TestEntity.NAME, after); assertValue.get(); } http://git-wip-us.apache.org/repos/asf/brooklyn-server/blob/2dcb0a03/core/src/test/java/org/apache/brooklyn/core/mgmt/internal/LocalSubscriptionManagerTest.java ---------------------------------------------------------------------- diff --git a/core/src/test/java/org/apache/brooklyn/core/mgmt/internal/LocalSubscriptionManagerTest.java b/core/src/test/java/org/apache/brooklyn/core/mgmt/internal/LocalSubscriptionManagerTest.java index 1373545..5c04978 100644 --- a/core/src/test/java/org/apache/brooklyn/core/mgmt/internal/LocalSubscriptionManagerTest.java +++ b/core/src/test/java/org/apache/brooklyn/core/mgmt/internal/LocalSubscriptionManagerTest.java @@ -193,8 +193,7 @@ public class LocalSubscriptionManagerTest extends BrooklynAppUnitTestSupport { // delivery should be in subscription order, so 123 then 456 entity.subscriptions().subscribe(ImmutableMap.of("notifyOfInitialValue", true), entity, TestEntity.SEQUENCE, listener); // wait for the above delivery - otherwise it might get dropped - Asserts.succeedsEventually(MutableMap.of("timeout", Duration.seconds(5)), () -> { - Asserts.assertSize(listener.getEvents(), 1); }); + Asserts.succeedsEventually(() -> Asserts.assertSize(listener.getEvents(), 1)); entity.sensors().set(TestEntity.SEQUENCE, 456); // notifications don't have "initial value" so don't get -1 @@ -207,8 +206,7 @@ public class LocalSubscriptionManagerTest extends BrooklynAppUnitTestSupport { entity.subscriptions().subscribe(ImmutableMap.of("notifyOfInitialValue", true), entity, TestEntity.SERVICE_STATE_ACTUAL, listener); entity.subscriptions().subscribe(ImmutableMap.of("notifyOfInitialValue", true), entity, TestEntity.NAME, listener); - Asserts.succeedsEventually(MutableMap.of("timeout", Duration.seconds(5)), new Runnable() { - @Override public void run() { + Asserts.succeedsEventually(() -> { Asserts.assertEquals(listener.getEvents(), ImmutableList.of( new BasicSensorEvent(TestEntity.SEQUENCE, entity, 123), new BasicSensorEvent(TestEntity.SEQUENCE, entity, 456), @@ -216,7 +214,7 @@ public class LocalSubscriptionManagerTest extends BrooklynAppUnitTestSupport { new BasicSensorEvent(TestEntity.SERVICE_STATE_ACTUAL, entity, Lifecycle.STOPPING), new BasicSensorEvent(TestEntity.NAME, entity, "myname")), "actually got: "+listener.getEvents()); - }}); + }); } @Test