Github user aledsage commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/816#discussion_r141592263
--- Diff:
core/src/main/java/org/apache/brooklyn/util/core/task/BasicExecutionManager.java
---
@@ -706,19 +719,23 @@ public void run() {
} else {
future = runner.submit(job);
}
+ afterSubmitRecordFuture(task, future);
+
+ return task;
+ }
+
+ protected <T> void afterSubmitRecordFuture(final Task<T> task,
Future<T> future) {
--- End diff --
TL;DR: an observation, rather than an action.
It feels like the task lifecycle is very complicated in our
`BasicExecutionManager`. Not saying that's anything to do with your PR (in fact
you've improved it by extracting code into explicit methods, rather than tagged
onto the end of things like `submitNewTask`). However, the number of `before*`
and `after*` methods, and the way listeners get called (e.g. in a nested
finally block inside `internalAfterEnd`) makes it clear that this is a very
complicated class!
---
