jcabrerizo commented on a change in pull request #1265:
URL: https://github.com/apache/brooklyn-server/pull/1265#discussion_r735507028



##########
File path: 
core/src/main/java/org/apache/brooklyn/util/core/task/ScheduledTask.java
##########
@@ -231,10 +231,12 @@ protected String getActiveTaskStatusString(int verbosity) 
{
     
     @Override
     public boolean isDone(boolean andTaskNoLongerRunning) {
+        boolean done = isCancelled() || (maxIterations!=null && maxIterations 
<= runCount) || (period==null && nextRun!=null && nextRun.isDone());

Review comment:
       I know you didn't change this but i wonder if `maxIterations <= 
runCount` should be the opposite: is done if `maxIterations > runCount`

##########
File path: 
rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/EntityResource.java
##########
@@ -173,46 +174,57 @@ public int compare(Task<?> o1, Task<?> o2) {
             if (!Objects.equal(o1.isSubmitted(), o2.isSubmitted())) {
                 return o1.isSubmitted() ? -1 : 1;
             }
-
-            // big pref for top-level tasks (manual operations), where 
submitter null
-            int weight = 0;
-            Task<?> o1s = o1.getSubmittedByTask();
-            Task<?> o2s = o2.getSubmittedByTask();
-            if ("start".equals(o1.getDisplayName()) 
||"start".equals(o2.getDisplayName())) {
-                weight = 0;
+            // followed by absolute pref for active items
+            if (!Objects.equal(o1.isDone(), o2.isDone())) {
+                return !o1.isDone() ? -1 : 1;

Review comment:
       I'd try not to do doble a negation here:
   ```
   return o1.isDone() ? 1 : -1;
   ```

##########
File path: 
utils/common/src/main/java/org/apache/brooklyn/util/guava/Functionals.java
##########
@@ -20,6 +20,8 @@
 
 import java.util.concurrent.Callable;
 
+import java.util.regex.Matcher;

Review comment:
       Are these import needed?

##########
File path: 
rest/rest-resources/src/main/java/org/apache/brooklyn/rest/resources/EntityResource.java
##########
@@ -173,46 +174,57 @@ public int compare(Task<?> o1, Task<?> o2) {
             if (!Objects.equal(o1.isSubmitted(), o2.isSubmitted())) {
                 return o1.isSubmitted() ? -1 : 1;
             }
-
-            // big pref for top-level tasks (manual operations), where 
submitter null
-            int weight = 0;
-            Task<?> o1s = o1.getSubmittedByTask();
-            Task<?> o2s = o2.getSubmittedByTask();
-            if ("start".equals(o1.getDisplayName()) 
||"start".equals(o2.getDisplayName())) {
-                weight = 0;
+            // followed by absolute pref for active items
+            if (!Objects.equal(o1.isDone(), o2.isDone())) {
+                return !o1.isDone() ? -1 : 1;
             }
-            if (!Objects.equal(o1s==null, o2s==null))
-                weight += 2*60*60 * (o1s==null ? -1 : 1);
-            
-            // pretty big pref for things invoked by other entities
-            if (context!=null && o1s!=null && o2s!=null) {
-                boolean o1se = 
context.equals(BrooklynTaskTags.getContextEntity(o1s));
-                boolean o2se = 
context.equals(BrooklynTaskTags.getContextEntity(o2s));
-                if (!Objects.equal(o1se, o2se))
-                    weight += 10*60 *  (o2se ? -1 : 1);
+
+            // followed by absolute pref for things not started yet
+            if (!Objects.equal(o1.isBegun(), o2.isBegun())) {
+                return !o1.isBegun() ? -1 : 1;

Review comment:
       same as above 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscr...@brooklyn.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to