brooklyn-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ahgittin <...@git.apache.org>
Subject [GitHub] incubator-brooklyn pull request: Implementation of proposed Test E...
Date Wed, 11 Nov 2015 09:31:18 GMT
Github user ahgittin commented on a diff in the pull request:

    https://github.com/apache/incubator-brooklyn/pull/999#discussion_r44514991
  
    --- Diff: usage/test-framework/src/main/java/org/apache/brooklyn/test/framework/ParallelTestCaseImpl.java
---
    @@ -0,0 +1,130 @@
    +package org.apache.brooklyn.test.framework;
    +
    +import java.util.Collection;
    +import java.util.concurrent.ExecutionException;
    +
    +import org.apache.brooklyn.api.location.Location;
    +import org.apache.brooklyn.api.mgmt.Task;
    +import org.apache.brooklyn.api.mgmt.TaskAdaptable;
    +import org.apache.brooklyn.core.entity.AbstractEntity;
    +import org.apache.brooklyn.core.entity.Attributes;
    +import org.apache.brooklyn.core.entity.lifecycle.Lifecycle;
    +import org.apache.brooklyn.core.entity.trait.StartableMethods;
    +import org.apache.brooklyn.util.core.task.DynamicTasks;
    +import org.apache.brooklyn.util.exceptions.Exceptions;
    +import org.slf4j.Logger;
    +import org.slf4j.LoggerFactory;
    +
    +/**
    + * This implementation will start all child entities in parallel.
    + * 
    + * @author Chris Burke
    + */
    +public class ParallelTestCaseImpl extends AbstractEntity implements ParallelTestCase
{
    +
    +    private static final Logger logger = LoggerFactory.getLogger(ParallelTestCaseImpl.class);
    +
    +    /**
    +     * {@inheritDoc}
    +     */
    +    public void start(Collection<? extends Location> locations) {
    +        // Let everyone know we're starting up (so that the GUI shows the correct icon).
    +        sensors().set(Attributes.SERVICE_STATE_ACTUAL, Lifecycle.STARTING);
    +        try {
    +            // Get an unsubmitted task for starting all the children of this entity in
parallel,
    +            // at the same location as this entity.
    +            final TaskAdaptable<?> taskAdaptable = StartableMethods.startingChildren(this);
    +            logger.trace("{}, TaskAdaptable: {}", this, taskAdaptable);
    +
    +            // Submit the task to the ExecutionManager so that they actually get started
    +            // and then wait until all the parallel child entities have completed.
    +            submitTaskAndWait(taskAdaptable);
    --- End diff --
    
    what you've done works but I'd probably do:
    
        DynamicTasks.queue(taskAdaptable);
        DynamicTasks.waitForLast();
    
    because it's an effector it's guaranteed to have a queueing context, and the advantage
of a queueing context is that you can build up more complicated task logic (e.g. starting
`target` if required) ahead of time and that task logic shows up nicely in the UI.  obviously
you want to wait for the queue to drain before setting the service state or catching, which
is what `waitForLast()` does, and it throws if there's a problem.  i wish we had better guidance
on tasks (though i think @sjcorbett wrote some?).
    
    more logging than we'd usually do as all the info will be available in the effector/task
logging, but no issues with that especially if it's helpful to learn.


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

Mime
View raw message