public inbox for pdm-devel@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal