You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by GitBox <gi...@apache.org> on 2022/11/29 08:09:10 UTC

[GitHub] [camel] orpiske commented on a diff in pull request #8785: CAMEL-18766 Fixes bug in camel-support

orpiske commented on code in PR #8785:
URL: https://github.com/apache/camel/pull/8785#discussion_r1034416912


##########
core/camel-support/src/main/java/org/apache/camel/support/task/BackgroundTask.java:
##########
@@ -167,12 +147,14 @@ private boolean waitForTaskCompletion(CountDownLatch latch, Future<?> task) {
         } finally {
             elapsed = budget.elapsed();
         }
-
-        return completed;
     }
 
     @Override
     public Duration elapsed() {
         return elapsed;
     }
+
+    private static class ExecutionResult {
+        private boolean completed;

Review Comment:
   Yes, it's stateful because of `budget` and `elapsed`.
   
   I think your suggestion is +/- in line with what I commented on the Zulip chat: to investigate moving the latch into the budget and consider it as part of that. Granted, I did not comment on the elapsed, but I think it would eventually become a natural move as well. 
   
   IMHO, making (all) the tasks stateless - keeping the state on the budget itself - could potentially be a better approach for the future (maybe as part of Camel 4?), but I think the current approach is fine.



-- 
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: commits-unsubscribe@camel.apache.org

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