[pdm-devel] [PATCH datacenter-manager] server: tasks: fix filter order

[pdm-devel] [PATCH datacenter-manager] server: tasks: fix filter order

[Dominik Csapak]

commit
 bd0babf (remote tasks: add background task for task polling, use new task cache)

changed the order of filters applied to the task list, namely it put the
skip and limit at the front of the filtering, instead of the back.

Since we use these paramters in the ui to batch loads, we don't want to
first limit our numbers and then filter on them, but first apply the
filters and then limit the amount, otherwise we could never show older
tasks when filtered.

An example:

Assume the ui usess 'limit=5' to batch the loads
if the user wants to show only those tasks with 'statusfilter=warning'

but we first have 10 'OK' tasks, we'd never show anything, since the api
would return an empty list, even if we'd have multiple warning tasks
after that.

In contrast if we filter first and then limit the number/apply the
offset, we would get the first five tasks that ended with a warning,
like we would expect.

To fix that, move the skip/take filters at the end.

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 server/src/remote_tasks/mod.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/server/src/remote_tasks/mod.rs b/server/src/remote_tasks/mod.rs
index 8638ebd..63a279a 100644
--- a/server/src/remote_tasks/mod.rs
+++ b/server/src/remote_tasks/mod.rs
@@ -38,8 +38,6 @@ pub async fn get_tasks(filters: TaskFilters) -> Result<Vec<TaskListItem>, Error>
 
         let returned_tasks = cache
             .get_tasks(which)?
-            .skip(filters.start as usize)
-            .take(filters.limit as usize)
             .filter_map(|task| {
                 // TODO: Handle PBS tasks
                 let pve_upid: Result<PveUpid, Error> = task.upid.upid.parse();
@@ -106,6 +104,8 @@ pub async fn get_tasks(filters: TaskFilters) -> Result<Vec<TaskListItem>, Error>
 
                 true
             })
+            .skip(filters.start as usize)
+            .take(filters.limit as usize)
             .collect();
 
         Ok(returned_tasks)
-- 
2.47.2



_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel

[pdm-devel] applied: [PATCH datacenter-manager] server: tasks: fix filter order

[Lukas Wagner]

On Fri Aug 22, 2025 at 2:11 PM CEST, Dominik Csapak wrote:
> commit
>  bd0babf (remote tasks: add background task for task polling, use new task cache)
>
> changed the order of filters applied to the task list, namely it put the
> skip and limit at the front of the filtering, instead of the back.
>
> Since we use these paramters in the ui to batch loads, we don't want to
> first limit our numbers and then filter on them, but first apply the
> filters and then limit the amount, otherwise we could never show older
> tasks when filtered.
>
> An example:
>
> Assume the ui usess 'limit=5' to batch the loads
> if the user wants to show only those tasks with 'statusfilter=warning'
>
> but we first have 10 'OK' tasks, we'd never show anything, since the api
> would return an empty list, even if we'd have multiple warning tasks
> after that.
>
> In contrast if we filter first and then limit the number/apply the
> offset, we would get the first five tasks that ended with a warning,
> like we would expect.
>
> To fix that, move the skip/take filters at the end.
>

Oops, good catch!

Looks good to me, applied :)


_______________________________________________
pdm-devel mailing list
pdm-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel