Github user ahgittin commented on a diff in the pull request:
https://github.com/apache/brooklyn-server/pull/816#discussion_r141630014
--- Diff:
core/src/main/java/org/apache/brooklyn/util/core/task/ValueResolver.java ---
@@ -434,7 +434,11 @@ protected boolean isEvaluatingImmediately() {
// (should discourage this in favour of task
factories which can be transiently interrupted?)
BrooklynTaskTags.addTagDynamically(task,
BrooklynTaskTags.NON_TRANSIENT_TASK_TAG);
}
- exec.submit(task);
+ if (timer!=null ||
Thread.currentThread().isInterrupted()) {
+ exec.submit(task);
+ } else {
+ exec.get(task);
--- End diff --
good spot on the `immediate(true)` entry point. i think how we resolve
that is the same as the question over `getImmediately(unsubmittedTask)`
semantics. will come back to this (as part of this review but not the initial
commit).
as for how we got here, immediate evaluation was retro-fitted on to many
different code paths -- some of which took `Supplier`, some which took `Task`,
others `Future`, `Callable` or even `Closure`. that's why the checks are here.
rather than immediate fail or do the wrong thing when given unexpected input,
we've built up logic (not accidentally!) to try to do the right thing, and
(also to the good) it is centralised here rather than all over. i think on
balance it is worth it to be able to do config validation early. it would be
good and feasible to cut down the types we use for "not-yet-known" values but
the more important improvement i think was centralising it here. venting is
understandable, i want to do it myself, but of course the criticism would be
more useful on the PR where the complexity was introduced rather than where I'm
attempting to clean it up. :)
---
