check for task completion by using task progress#2290
Conversation
…king task progress and not keyspaceprogress
There was a problem hiding this comment.
Pull request overview
This PR addresses #2270 by making task “completion”/status reflect searched progress rather than distributed keyspace progress, so tasks don’t appear completed (green) immediately after distribution reaches 100%.
Changes:
- Update aggregate status computation to pass task progress (searched) into
TaskUtils::getStatus()instead ofkeyspaceProgress(distributed). - Simplify
TaskUtils::getStatus()“running” detection by relying on recent chunk activity (dispatch/solve time). - Apply the updated status calculation in both task and task-wrapper v2 API models.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/inc/utils/TaskUtils.php | Adjusts the “running” condition used when deriving a task status from chunk activity. |
| src/inc/apiv2/model/TaskWrapperDisplayAPI.php | Uses searched progress (getTaskProgress) when calculating per-task status for wrapper aggregation. |
| src/inc/apiv2/model/TaskAPI.php | Uses searched progress (getTaskProgress) when calculating aggregate task status. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
I agree with copilot on its comment about removing the check for progress < 10000, currently indeed the function only is called by filtering before, but the function may be used at some other place later.
What probably would be the best way is to integrate the query to get the chunks itself into the function getStatus(), so that you only pass the Task object as argument and everything else is done inside the function. This avoids that mistake calls can be made to the function (with not the right chunks filtered out) and also reduces code duplication.
closes #2270