all lists on lists.proxmox.com
 help / color / mirror / Atom feed
* SPAM: [PATCH datacenter-manager] api: local/remote tasks: simplify errors/statusfilter logic
@ 2026-08-31 11:45 Lukas Wagner
  2026-08-31 12:21 ` Jonas Theisen
  0 siblings, 1 reply; 2+ messages in thread
From: Lukas Wagner @ 2026-08-31 11:45 UTC (permalink / raw)
  To: pdm-devel

`errors` is semantically a subset of `statusfilter`, so we can simplify
the code quite a bit if we include the behavior controlled by `errors`
into `statusfilter` and then only filter based on the latter.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 server/src/api/nodes/tasks.rs  | 31 +++++++++++++++++++++++--------
 server/src/remote_tasks/mod.rs | 26 +++++++++++++++++---------
 2 files changed, 40 insertions(+), 17 deletions(-)

diff --git a/server/src/api/nodes/tasks.rs b/server/src/api/nodes/tasks.rs
index 1cca3c0a..7f96656a 100644
--- a/server/src/api/nodes/tasks.rs
+++ b/server/src/api/nodes/tasks.rs
@@ -88,6 +88,16 @@ pub fn list_tasks(
         statusfilter,
     } = filters;
 
+    let mut statusfilter = statusfilter.unwrap_or_default();
+    if errors {
+        // Contrary to popular belief, `errors` returns more than just errors...
+        statusfilter.extend([
+            TaskStateType::Error,
+            TaskStateType::Unknown,
+            TaskStateType::Warning,
+        ]);
+    }
+
     let auth_id: Authid = rpcenv
         .get_auth_id()
         .context("no authid available")?
@@ -147,17 +157,22 @@ pub fn list_tasks(
             }
         }
 
-        match (&info.state, &statusfilter) {
-            (Some(_), _) if running => continue,
-            (Some(TaskState::OK { .. }), _) if errors => continue,
-            (Some(state), Some(filters)) => {
-                if !filters.contains(&tasktype(state)) {
+        if info.state.is_some() && running {
+            // if there is a task state and we filter by `running`, don't return the task
+            continue;
+        }
+
+        if !statusfilter.is_empty() {
+            if let Some(state) = &info.state {
+                if !statusfilter.contains(&tasktype(state)) {
+                    // A finished task is filtered out if its state is not contained in
+                    // the list of allowed values
                     continue;
                 }
+            } else {
+                // an unfinished task is filtered out if *any* status filter is set
+                continue;
             }
-            (None, _) if errors => continue,
-            (None, Some(_)) => continue,
-            _ => {}
         }
 
         if skipped < start as usize {
diff --git a/server/src/remote_tasks/mod.rs b/server/src/remote_tasks/mod.rs
index bcae7f4c..05a875d6 100644
--- a/server/src/remote_tasks/mod.rs
+++ b/server/src/remote_tasks/mod.rs
@@ -51,6 +51,16 @@ pub async fn get_tasks(
             limit => limit as usize,
         };
 
+        let mut status_filter = filters.statusfilter.unwrap_or_default();
+        if filters.errors {
+            // Contrary to popular belief, `errors` returns more than just errors...
+            status_filter.extend([
+                TaskStateType::Error,
+                TaskStateType::Unknown,
+                TaskStateType::Warning,
+            ]);
+        }
+
         let returned_tasks = cache
             .get_tasks(which)?
             .filter_map(|task| {
@@ -140,16 +150,14 @@ pub async fn get_tasks(
 
                 let state = item.status.as_deref().map(TaskStateType::new_from_str);
 
-                match (state, &filters.statusfilter) {
-                    (Some(TaskStateType::OK), _) if filters.errors => return false,
-                    (Some(state), Some(filters)) => {
-                        if !filters.contains(&state) {
-                            return false;
-                        }
+                if !status_filter.is_empty() {
+                    if let Some(state) = &state {
+                        // Only apply status filters against finished tasks.
+                        return status_filter.contains(state);
+                    } else {
+                        // If status filters are set, a running task should not be returned.
+                        return false;
                     }
-                    (None, Some(_)) => return false,
-                    (None, _) if filters.errors => return false,
-                    _ => {}
                 }
 
                 true
-- 
2.47.3





^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: SPAM: [PATCH datacenter-manager] api: local/remote tasks: simplify errors/statusfilter logic
  2026-08-31 11:45 SPAM: [PATCH datacenter-manager] api: local/remote tasks: simplify errors/statusfilter logic Lukas Wagner
@ 2026-08-31 12:21 ` Jonas Theisen
  0 siblings, 0 replies; 2+ messages in thread
From: Jonas Theisen @ 2026-08-31 12:21 UTC (permalink / raw)
  To: pdm-devel

On 8/31/26 13:45, Lukas Wagner wrote:
> `errors` is semantically a subset of `statusfilter`, so we can simplify
> the code quite a bit if we include the behavior controlled by `errors`
> into `statusfilter` and then only filter based on the latter.
>
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
>
> -snip-

Tested the filters with the patch on both Remotes -> Tasks
and {Nodename} -> Tasks and works fine on both pages.
Also tested combinations with "Task type" filters and works
as expected.

For reference PDM is configured with a PVE and a PBS remote.
--
Tested-by: Jonas Theisen <j.theisen@proxmox.com>




^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-08-31 12:21 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-31 11:45 SPAM: [PATCH datacenter-manager] api: local/remote tasks: simplify errors/statusfilter logic Lukas Wagner
2026-08-31 12:21 ` Jonas Theisen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal