Re: [pdm-devel] [RFC PATCH datacenter-manager v2 15/16] ui: dashboard: implement 'View'

["Shannon Sterz" <s.sterz@proxmox.com>]

[-- Attachment #1.1: Type: text/plain, Size: 1986 bytes --]

On 10/23/25 1:44 PM, Dominik Csapak wrote:
> On 10/23/25 1:19 PM, Shannon Sterz wrote:
>> On Thu Oct 23, 2025 at 10:28 AM CEST, Dominik Csapak wrote:
> [snip]
>>> +
>>> +struct ViewComp {
>>> +    template: LoadResult<ViewTemplate, Error>,
>>> +
>>> +    // various api call results
>>> +    status: LoadResult<ResourcesStatus, Error>,
>>> +    top_entities: LoadResult<TopEntities, proxmox_client::Error>,
>>> +    statistics: LoadResult<TaskStatistics, Error>,
>>
>> this is fine, but i just had an idea, maybe this isn't too useful right
>> now, but might be worth exploring: we could turn this into a HashMap
>> with something like this:
>>
>> HashMap<ToQuery, LoadResult<ApiResponseData, Error>>
>>
>> then loading could become iterating over the keys and calling a function
>> on them. with a wrapper type we could even implement a getter that
>> transforms the ApiResponseData to a concrete type. might cut down on the
>> loading logic below and make this more easily extensible in the future.
>>
>> the required_api_calls below could then just return such a hashmap with
>> only the necessary keys. what do you think (note i haven't tested any of
>> this)?
>
> i don't think this will work, since ApiResponseData itself takes a
> generic parameter too, and we can't use different ones for different
> values of the same hashmap AFAIK
>
> but yeah, we should think about how we could generalize this
> instead of just adding on new members...
>

sorry seems I dropped the list in my last response... so sending this
again.

thought about this a bit. we can leverage DeserializeOwned and Value
here quite a bit to make it work. i did a bit of messing around and came
up with the attached patch. this certainly isn't perfect or polished
(e.g. we could use wrapper types to move the get_entities function
somewhere more sensible; safe a clone here or there etc.), but the
general gist is there. it also works so yeah :)


[-- Attachment #2: 0001-wip-ui-view-refactor-loading-logic-to-use-hashmap.patch --]
[-- Type: text/x-patch, Size: 16051 bytes --]

From e266584fc8efa4504135e955c4e64ae7abda27b4 Mon Sep 17 00:00:00 2001
From: Shannon Sterz <s.sterz@proxmox.com>
Date: Fri, 24 Oct 2025 12:10:41 +0200
Subject: [PATCH] wip: ui: view: refactor loading logic to use hashmap

this is mostly just a poc that we could use a hashmap of loadable
entities instead of having to specify a field for each separately.
---
 server/src/metric_collection/top_entities.rs |   2 +-
 ui/src/dashboard/top_entities.rs             |   2 +-
 ui/src/dashboard/view.rs                     | 222 +++++++++++--------
 3 files changed, 128 insertions(+), 98 deletions(-)

diff --git a/server/src/metric_collection/top_entities.rs b/server/src/metric_collection/top_entities.rs
index ea121ee..73a3e63 100644
--- a/server/src/metric_collection/top_entities.rs
+++ b/server/src/metric_collection/top_entities.rs
@@ -36,7 +36,7 @@ pub fn calculate_top(
     remotes: &HashMap<String, pdm_api_types::remotes::Remote>,
     timeframe: proxmox_rrd_api_types::RrdTimeframe,
     num: usize,
-    check_remote_privs: impl Fn(&str) -> bool
+    check_remote_privs: impl Fn(&str) -> bool,
 ) -> TopEntities {
     let mut guest_cpu = Vec::new();
     let mut node_cpu = Vec::new();
diff --git a/ui/src/dashboard/top_entities.rs b/ui/src/dashboard/top_entities.rs
index dfe3869..25e62c4 100644
--- a/ui/src/dashboard/top_entities.rs
+++ b/ui/src/dashboard/top_entities.rs
@@ -328,7 +328,7 @@ fn graph_from_data(data: &Vec<Option<f64>>, threshold: f64) -> Container {
 
 pub fn create_top_entities_panel(
     entities: Option<Vec<TopEntity>>,
-    error: Option<&proxmox_client::Error>,
+    error: Option<&anyhow::Error>,
     leaderboard_type: LeaderboardType,
 ) -> Panel {
     let (icon, title, metrics_title, threshold) = match leaderboard_type {
diff --git a/ui/src/dashboard/view.rs b/ui/src/dashboard/view.rs
index 6cea9f8..80d1c55 100644
--- a/ui/src/dashboard/view.rs
+++ b/ui/src/dashboard/view.rs
@@ -1,10 +1,14 @@
+use std::collections::HashMap;
+use std::hash::Hash;
 use std::rc::Rc;
 
-use anyhow::Error;
-use futures::join;
+use anyhow::{format_err, Error};
+use futures::future::select_all;
+use html::Scope;
 use js_sys::Date;
-use pwt::widget::form::FormContext;
+use serde::de::DeserializeOwned;
 use serde_json::json;
+use serde_json::Value;
 use yew::virtual_dom::{VComp, VNode};
 
 use proxmox_yew_comp::http_get;
@@ -12,6 +16,7 @@ use pwt::css;
 use pwt::prelude::*;
 use pwt::props::StorageLocation;
 use pwt::state::PersistentState;
+use pwt::widget::form::FormContext;
 use pwt::widget::{error_message, Column, Container, Panel, Progress, Row};
 use pwt::AsyncPool;
 
@@ -32,8 +37,6 @@ use crate::remotes::AddWizard;
 use crate::{pdm_client, LoadResult};
 
 use pdm_api_types::remotes::RemoteType;
-use pdm_api_types::resource::ResourcesStatus;
-use pdm_api_types::TaskStatistics;
 use pdm_client::types::TopEntities;
 
 #[derive(Properties, PartialEq)]
@@ -54,29 +57,67 @@ impl View {
     }
 }
 
-pub enum LoadingResult {
-    Resources(Result<ResourcesStatus, Error>),
-    TopEntities(Result<pdm_client::types::TopEntities, proxmox_client::Error>),
-    TaskStatistics(Result<TaskStatistics, Error>),
-    All,
-}
-
 pub enum Msg {
     ViewTemplateLoaded(Result<ViewTemplate, Error>),
-    LoadingResult(LoadingResult),
+    LoadingResult((LoadableEntities, Result<Value, Error>)),
     CreateWizard(Option<RemoteType>),
     Reload(bool),       // force
     ConfigWindow(bool), // show
     UpdateConfig(RefreshConfig),
+    LoadingDone,
+}
+
+#[derive(Hash, PartialEq, Eq, Debug, Clone, Copy)]
+pub enum LoadableEntities {
+    Status,
+    TopEntities,
+    Statistics,
+}
+
+impl LoadableEntities {
+    async fn load(&self, max_age: u64, since: i64, link: &Scope<ViewComp>) {
+        let res = match self {
+            LoadableEntities::Status => {
+                http_get("/resources/status", Some(json!({"max-age": max_age}))).await
+            }
+            LoadableEntities::TopEntities => {
+                let client: pdm_client::PdmClient<Rc<proxmox_yew_comp::HttpClientWasm>> =
+                    pdm_client();
+                client
+                    .get_top_entities()
+                    .await
+                    .map(|r| serde_json::to_value(r).unwrap())
+                    .map_err(|e| format_err!("could not load top entities").context(e))
+            }
+            LoadableEntities::Statistics => {
+                let params = Some(json!({
+                    "since": since,
+                    "limit": 0,
+                }));
+                http_get("/remote-tasks/statistics", params).await
+            }
+        };
+
+        link.send_message(Msg::LoadingResult((*self, res)));
+    }
+}
+
+fn get_entity<T: DeserializeOwned>(
+    map: &HashMap<LoadableEntities, LoadResult<Value, Error>>,
+    key: &LoadableEntities,
+) -> Option<T> {
+    map.get(key).and_then(|d| {
+        d.data
+            .as_ref()
+            .and_then(|d| serde_json::from_value(d.clone()).ok())
+    })
 }
 
 struct ViewComp {
     template: LoadResult<ViewTemplate, Error>,
 
     // various api call results
-    status: LoadResult<ResourcesStatus, Error>,
-    top_entities: LoadResult<TopEntities, proxmox_client::Error>,
-    statistics: LoadResult<TaskStatistics, Error>,
+    load_results: HashMap<LoadableEntities, LoadResult<Value, Error>>,
 
     refresh_config: PersistentState<RefreshConfig>,
 
@@ -90,14 +131,16 @@ struct ViewComp {
 impl ViewComp {
     fn create_widget(&self, ctx: &yew::Context<Self>, widget: &WidgetType) -> Panel {
         match widget {
-            WidgetType::Nodes { remote_type } => {
-                create_node_panel(*remote_type, self.status.data.clone())
-            }
-            WidgetType::Guests { guest_type } => {
-                create_guest_panel(*guest_type, self.status.data.clone())
-            }
+            WidgetType::Nodes { remote_type } => create_node_panel(
+                *remote_type,
+                get_entity(&self.load_results, &LoadableEntities::Status),
+            ),
+            WidgetType::Guests { guest_type } => create_guest_panel(
+                *guest_type,
+                get_entity(&self.load_results, &LoadableEntities::Status),
+            ),
             WidgetType::Remotes { show_wizard } => create_remote_panel(
-                self.status.data.clone(),
+                get_entity(&self.load_results, &LoadableEntities::Status),
                 show_wizard.then_some(
                     ctx.link()
                         .callback(|_| Msg::CreateWizard(Some(RemoteType::Pve))),
@@ -108,28 +151,29 @@ impl ViewComp {
                 ),
             ),
             WidgetType::Subscription => create_subscription_panel(),
-            WidgetType::Sdn => create_sdn_panel(self.status.data.clone()),
+            WidgetType::Sdn => {
+                create_sdn_panel(get_entity(&self.load_results, &LoadableEntities::Status))
+            }
             WidgetType::Leaderboard { leaderboard_type } => {
+                let top_entities: Option<TopEntities> =
+                    get_entity(&self.load_results, &LoadableEntities::TopEntities);
+
                 let entities = match leaderboard_type {
-                    LeaderboardType::GuestCpu => self
-                        .top_entities
-                        .data
+                    LeaderboardType::GuestCpu => top_entities
                         .as_ref()
                         .map(|entities| entities.guest_cpu.clone()),
-                    LeaderboardType::NodeCpu => self
-                        .top_entities
-                        .data
+                    LeaderboardType::NodeCpu => top_entities
                         .as_ref()
                         .map(|entities| entities.node_cpu.clone()),
-                    LeaderboardType::NodeMemory => self
-                        .top_entities
-                        .data
+                    LeaderboardType::NodeMemory => top_entities
                         .as_ref()
                         .map(|entities| entities.node_memory.clone()),
                 };
                 create_top_entities_panel(
                     entities,
-                    self.top_entities.error.as_ref(),
+                    self.load_results
+                        .get(&LoadableEntities::TopEntities)
+                        .and_then(|t| t.error.as_ref()),
                     *leaderboard_type,
                 )
             }
@@ -140,8 +184,10 @@ impl ViewComp {
                 };
                 let (hours, since) = get_task_options(self.refresh_config.task_last_hours);
                 create_task_summary_panel(
-                    self.statistics.data.clone(),
-                    self.statistics.error.as_ref(),
+                    get_entity(&self.load_results, &LoadableEntities::Statistics),
+                    self.load_results
+                        .get(&LoadableEntities::Statistics)
+                        .and_then(|t| t.error.as_ref()),
                     remotes,
                     hours,
                     since,
@@ -160,56 +206,41 @@ impl ViewComp {
     }
 
     fn do_reload(&mut self, ctx: &yew::Context<Self>, max_age: u64) {
-        if let Some(data) = self.template.data.as_ref() {
+        if self.template.data.as_ref().is_some() {
             let link = ctx.link().clone();
             let (_, since) = get_task_options(self.refresh_config.task_last_hours);
-            let (status, top_entities, tasks) = required_api_calls(&data.layout);
+            let keys = self
+                .load_results
+                .keys()
+                .cloned()
+                .collect::<Vec<LoadableEntities>>();
 
             self.loading = true;
+
             self.async_pool.spawn(async move {
-                let status_future = async {
-                    if status {
-                        let res =
-                            http_get("/resources/status", Some(json!({"max-age": max_age}))).await;
-                        link.send_message(Msg::LoadingResult(LoadingResult::Resources(res)));
-                    }
-                };
+                let mut futures = Vec::new();
 
-                let entities_future = async {
-                    if top_entities {
-                        let client: pdm_client::PdmClient<Rc<proxmox_yew_comp::HttpClientWasm>> =
-                            pdm_client();
-                        let res = client.get_top_entities().await;
-                        link.send_message(Msg::LoadingResult(LoadingResult::TopEntities(res)));
-                    }
-                };
+                for key in keys {
+                    let key = key.clone();
+                    let link = link.clone();
+                    let future = Box::pin(async move { key.load(max_age, since, &link).await });
+                    futures.push(future);
+                }
 
-                let tasks_future = async {
-                    if tasks {
-                        let params = Some(json!({
-                            "since": since,
-                            "limit": 0,
-                        }));
-                        let res = http_get("/remote-tasks/statistics", params).await;
-                        link.send_message(Msg::LoadingResult(LoadingResult::TaskStatistics(res)));
-                    }
-                };
-
-                join!(status_future, entities_future, tasks_future);
-                link.send_message(Msg::LoadingResult(LoadingResult::All));
+                select_all(futures).await;
+                link.send_message(Msg::LoadingDone);
             });
         } else {
-            ctx.link()
-                .send_message(Msg::LoadingResult(LoadingResult::All));
+            ctx.link().send_message(Msg::LoadingDone);
         }
     }
 }
 
 // returns which api calls are required: status, top_entities, task statistics
-fn required_api_calls(layout: &ViewLayout) -> (bool, bool, bool) {
-    let mut status = false;
-    let mut top_entities = false;
-    let mut task_statistics = false;
+fn required_api_calls(
+    map: &mut HashMap<LoadableEntities, LoadResult<Value, Error>>,
+    layout: &ViewLayout,
+) {
     match layout {
         ViewLayout::Rows { rows } => {
             for row in rows {
@@ -219,20 +250,22 @@ fn required_api_calls(layout: &ViewLayout) -> (bool, bool, bool) {
                         | WidgetType::Guests { .. }
                         | WidgetType::Remotes { .. }
                         | WidgetType::Sdn => {
-                            status = true;
+                            map.insert(LoadableEntities::Status, LoadResult::new());
                         }
                         WidgetType::Subscription => {
                             // panel does it itself, it's always required anyway
                         }
-                        WidgetType::Leaderboard { .. } => top_entities = true,
-                        WidgetType::TaskSummary { .. } => task_statistics = true,
+                        WidgetType::Leaderboard { .. } => {
+                            map.insert(LoadableEntities::TopEntities, LoadResult::new());
+                        }
+                        WidgetType::TaskSummary { .. } => {
+                            map.insert(LoadableEntities::Statistics, LoadResult::new());
+                        }
                     }
                 }
             }
         }
     }
-
-    (status, top_entities, task_statistics)
 }
 
 fn has_sub_panel(layout: Option<&ViewTemplate>) -> bool {
@@ -269,10 +302,7 @@ impl Component for ViewComp {
         Self {
             template: LoadResult::new(),
             async_pool,
-
-            status: LoadResult::new(),
-            top_entities: LoadResult::new(),
-            statistics: LoadResult::new(),
+            load_results: HashMap::new(),
 
             refresh_config,
             load_finished_time: None,
@@ -286,24 +316,24 @@ impl Component for ViewComp {
         match msg {
             Msg::ViewTemplateLoaded(view_template) => {
                 self.template.update(view_template);
+                if let Some(template) = self.template.data.as_ref() {
+                    required_api_calls(&mut self.load_results, &template.layout);
+                }
                 self.reload(ctx);
             }
-            Msg::LoadingResult(loading_result) => match loading_result {
-                LoadingResult::Resources(status) => self.status.update(status),
-                LoadingResult::TopEntities(top_entities) => self.top_entities.update(top_entities),
-                LoadingResult::TaskStatistics(task_statistics) => {
-                    self.statistics.update(task_statistics)
+            Msg::LoadingResult((entity, result)) => {
+                self.load_results.get_mut(&entity).unwrap().update(result)
+            }
+            Msg::LoadingDone => {
+                self.loading = false;
+                if self.load_finished_time.is_none() {
+                    // immediately trigger a "normal" reload after the first load with the
+                    // configured or default max-age to ensure users sees more current data.
+                    ctx.link().send_message(Msg::Reload(false));
                 }
-                LoadingResult::All => {
-                    self.loading = false;
-                    if self.load_finished_time.is_none() {
-                        // immediately trigger a "normal" reload after the first load with the
-                        // configured or default max-age to ensure users sees more current data.
-                        ctx.link().send_message(Msg::Reload(false));
-                    }
-                    self.load_finished_time = Some(Date::now() / 1000.0);
-                }
-            },
+                self.load_finished_time = Some(Date::now() / 1000.0);
+            }
+
             Msg::CreateWizard(remote_type) => {
                 self.show_create_wizard = remote_type;
             }
-- 
2.47.3


[-- Attachment #3: Type: text/plain, Size: 160 bytes --]

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