You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@brooklyn.apache.org by sjcorbett <gi...@git.apache.org> on 2015/08/05 18:12:15 UTC
[GitHub] incubator-brooklyn pull request: BasicStartable entities transitio...
GitHub user sjcorbett opened a pull request:
https://github.com/apache/incubator-brooklyn/pull/798
BasicStartable entities transition through expected lifecycle states
And adds useful test class `RecordingSensorEventListener`.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/sjcorbett/incubator-brooklyn fix/basicstartable-lifecycle
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-brooklyn/pull/798.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 #798
----
commit a18d86032703bb2519ed703419b4eed783ea84a3
Author: Sam Corbett <sa...@cloudsoftcorp.com>
Date: 2015-07-20T18:25:59Z
BasicStartableImpl sets expected lifecycle states
commit 29dfa210a7b33c729fad49cde9643a9c50f4cce5
Author: Sam Corbett <sa...@cloudsoftcorp.com>
Date: 2015-08-05T14:55:03Z
Test lifecycle phases of BasicStartable
commit 744bc451299a5cec85aa0ed1797387d6cc94c0bc
Author: Sam Corbett <sa...@cloudsoftcorp.com>
Date: 2015-08-05T15:04:21Z
Replace many versions of RecordingSensorEventListener
----
---
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: BasicStartable entities transitio...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-brooklyn/pull/798
---
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: BasicStartable entities transitio...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/798#discussion_r36416514
--- Diff: core/src/main/java/brooklyn/entity/basic/BasicStartableImpl.java ---
@@ -38,38 +39,47 @@
public class BasicStartableImpl extends AbstractEntity implements BasicStartable {
private static final Logger log = LoggerFactory.getLogger(BasicStartableImpl.class);
-
- public BasicStartableImpl() {
- super();
- }
@Override
public void start(Collection<? extends Location> locations) {
log.info("Starting entity "+this+" at "+locations);
- addLocations(locations);
-
- // essentially does StartableMethods.start(this, locations),
- // but optionally filters locations for each child
-
- brooklyn.location.basic.Locations.LocationsFilter filter = getConfig(LOCATIONS_FILTER);
- Iterable<Entity> startables = filterStartableManagedEntities(getChildren());
- if (startables == null || Iterables.isEmpty(startables)) return;
+ try {
+ ServiceStateLogic.setExpectedState(this, Lifecycle.STARTING);
+
+ addLocations(locations);
+
+ // essentially does StartableMethods.start(this, locations),
+ // but optionally filters locations for each child
- List<Task<?>> tasks = Lists.newArrayList();
- for (final Entity entity : startables) {
- Collection<? extends Location> l2 = locations;
- if (filter!=null) {
- l2 = filter.filterForContext(new ArrayList<Location>(locations), entity);
- log.debug("Child "+entity+" of "+this+" being started in filtered location list: "+l2);
+ brooklyn.location.basic.Locations.LocationsFilter filter = getConfig(LOCATIONS_FILTER);
+ Iterable<Entity> startables = filterStartableManagedEntities(getChildren());
+ if (!Iterables.isEmpty(startables)) {
+ List<Task<?>> tasks = Lists.newArrayListWithCapacity(Iterables.size(startables));
+ for (final Entity entity : startables) {
+ Collection<? extends Location> l2 = locations;
+ if (filter != null) {
+ l2 = filter.filterForContext(new ArrayList<Location>(locations), entity);
+ log.debug("Child " + entity + " of " + this + " being started in filtered location list: " + l2);
+ }
+ tasks.add(Entities.invokeEffectorWithArgs(this, entity, Startable.START, l2));
+ }
+ for (Task<?> t : tasks) {
+ t.getUnchecked();
+ }
}
- tasks.add( Entities.invokeEffectorWithArgs(this, entity, Startable.START, l2) );
+ setAttribute(Attributes.SERVICE_UP, true);
+ ServiceStateLogic.setExpectedState(this, Lifecycle.RUNNING);
+ } catch (Throwable t) {
+ ServiceStateLogic.setExpectedState(this, Lifecycle.ON_FIRE);
+ throw Exceptions.propagate(t);
}
- for (Task<?> t: tasks) t.getUnchecked();
}
@Override
public void stop() {
+ ServiceStateLogic.setExpectedState(this, Lifecycle.STOPPING);
StartableMethods.stop(this);
--- End diff --
should this be wrapped in a try-catch, so that we can do `ServiceStateLogic.setExpectedState(this, Lifecycle.ON_FIRE);`? For example, see `DynamicClusterImpl.stop()` and `AbstractApplication.stop()`.
---
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: BasicStartable entities transitio...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on a diff in the pull request:
https://github.com/apache/incubator-brooklyn/pull/798#discussion_r36417201
--- Diff: core/src/test/java/org/apache/brooklyn/entity/basic/RecordingSensorEventListener.java ---
@@ -0,0 +1,108 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.brooklyn.entity.basic;
+
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Objects;
+
+import com.google.common.base.Function;
+import com.google.common.collect.FluentIterable;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Lists;
+import com.google.common.primitives.Longs;
+
+import brooklyn.event.SensorEvent;
+import brooklyn.event.SensorEventListener;
+
+/**
+ * An event listener that records each event and allows callers to access all values and
+ * all values sorted by event timestamp.
+ */
+public class RecordingSensorEventListener<T> implements SensorEventListener<T>, Iterable<SensorEvent<T>> {
+
+ private final List<SensorEvent<T>> events = Lists.newCopyOnWriteArrayList();
+ private final boolean suppressDuplicates;
+ private T lastValue;
+
+ public RecordingSensorEventListener() {
+ this(false);
+ }
+
+ public RecordingSensorEventListener(boolean suppressDuplicates) {
+ this.suppressDuplicates = suppressDuplicates;
+ }
+
+ @Override
+ public void onEvent(SensorEvent<T> event) {
+ if (!suppressDuplicates || events.isEmpty() || !Objects.equals(lastValue, event.getValue())) {
+ events.add(event);
+ lastValue = event.getValue();
+ }
+ }
+
+ /**
+ * @return An immutable iterable of the recorded events.
+ */
+ public Iterable<SensorEvent<T>> getEvents() {
+ return ImmutableList.copyOf(events);
+ }
+
+ public Iterable<T> getEventValues() {
--- End diff --
Deserves a comment that this is a live (read-only) view of the event values. At first I didn't see how this worked with `Suppliers.ofInstance(listener.getEventValues())`, but after looking at what a `FluentIterable`'s `iterator()` method does, this is an elegant solution!
---
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: BasicStartable entities transitio...
Posted by sjcorbett <gi...@git.apache.org>.
Github user sjcorbett commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/798#issuecomment-128404113
@aledsage Thanks for your comments. In #790 @alasdairhodge raised the idea of having the listener in the test-support module. I'm happy to make that change if considered appropriate.
---
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: BasicStartable entities transitio...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/798#issuecomment-128428791
Thanks @sjcorbett - I'm happy to merge now. We can discuss separately if that listener should be in test-support.
---
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: BasicStartable entities transitio...
Posted by aledsage <gi...@git.apache.org>.
Github user aledsage commented on the pull request:
https://github.com/apache/incubator-brooklyn/pull/798#issuecomment-128376435
Looks great. Only a couple of very minor comments. Good to merge once you've looked at those.
---
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.
---