From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id A14DF1FF165 for ; Thu, 23 Oct 2025 13:19:45 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 240D47E8A; Thu, 23 Oct 2025 13:20:13 +0200 (CEST) Mime-Version: 1.0 Date: Thu, 23 Oct 2025 13:19:35 +0200 Message-Id: To: "Dominik Csapak" X-Mailer: aerc 0.20.0 References: <20251023083253.1038119-1-d.csapak@proxmox.com> <20251023083253.1038119-15-d.csapak@proxmox.com> In-Reply-To: <20251023083253.1038119-15-d.csapak@proxmox.com> From: "Shannon Sterz" X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1761218367664 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.056 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pdm-devel] [RFC PATCH datacenter-manager v2 14/16] ui: introduce `LoadResult` helper type X-BeenThere: pdm-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Datacenter Manager development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox Datacenter Manager development discussion Cc: Proxmox Datacenter Manager development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pdm-devel-bounces@lists.proxmox.com Sender: "pdm-devel" On Thu Oct 23, 2025 at 10:28 AM CEST, Dominik Csapak wrote: > this factors out some common pattern when loading data, such as saving > the last valid data even when an error occurs, and a check if anything > has been set yet. > > This saves a few lines when we use it vs duplicating that pattern > everywhere. > this is a nice idea, i'd support putting this in pwt for sure as this is a super common pattern. putting it in pdm and then latter switching to the one from pwt might mean a lot of churn, though. > Signed-off-by: Dominik Csapak > --- > changes from v1: > * correctly mark as RFC > > ui/src/lib.rs | 3 +++ > ui/src/load_result.rs | 42 +++++++++++++++++++++++++++++++++++++ > ui/src/pbs/remote.rs | 30 +++++++++----------------- > ui/src/pve/lxc/overview.rs | 28 +++++++------------------ > ui/src/pve/node/overview.rs | 29 +++++++++---------------- > ui/src/pve/qemu/overview.rs | 28 +++++++------------------ > ui/src/pve/storage.rs | 29 +++++++------------------ > 7 files changed, 89 insertions(+), 100 deletions(-) > create mode 100644 ui/src/load_result.rs > > diff --git a/ui/src/lib.rs b/ui/src/lib.rs > index a2b79b05..de76e1c0 100644 > --- a/ui/src/lib.rs > +++ b/ui/src/lib.rs > @@ -39,6 +39,9 @@ pub mod sdn; > > pub mod renderer; > > +mod load_result; > +pub use load_result::LoadResult; > + > mod tasks; > pub use tasks::register_pve_tasks; > > diff --git a/ui/src/load_result.rs b/ui/src/load_result.rs > new file mode 100644 > index 00000000..4f3e6d5a > --- /dev/null > +++ b/ui/src/load_result.rs > @@ -0,0 +1,42 @@ > +/// Helper wrapper to factor out some common api loading behavior > +pub struct LoadResult { > + pub data: Option, > + pub error: Option, > +} > + > +impl LoadResult { > + /// Creates a new empty result that contains no data or error. > + pub fn new() -> Self { > + Self { > + data: None, > + error: None, > + } > + } > + > + /// Update the current value with the given result > + /// > + /// On `Ok`, the previous error will be deleted. > + /// On `Err`, the previous valid date is kept. > + pub fn update(&mut self, result: Result) { > + match result { > + Ok(data) => { > + self.error = None; > + self.data = Some(data); > + } > + Err(err) => { > + self.error = Some(err); > + } > + } > + } > + > + /// If any of data or err has any value > + pub fn has_data(&self) -> bool { > + self.data.is_some() || self.error.is_some() > + } > +} > + > +impl Default for LoadResult { > + fn default() -> Self { > + Self::new() > + } > +} > diff --git a/ui/src/pbs/remote.rs b/ui/src/pbs/remote.rs > index 7cf7c7e2..67c9172f 100644 > --- a/ui/src/pbs/remote.rs > +++ b/ui/src/pbs/remote.rs > @@ -17,7 +17,7 @@ use pwt::{ > use pbs_api_types::NodeStatus; > use pdm_api_types::rrddata::PbsNodeDataPoint; > > -use crate::renderer::separator; > +use crate::{renderer::separator, LoadResult}; > > #[derive(Clone, Debug, Eq, PartialEq, Properties)] > pub struct RemoteOverviewPanel { > @@ -59,12 +59,11 @@ pub struct RemoteOverviewPanelComp { > load_data: Rc, > mem_data: Rc, > mem_total_data: Rc, > - status: Option, > + status: LoadResult, > > rrd_time_frame: RRDTimeframe, > > last_error: Option, > - last_status_error: Option, > > async_pool: AsyncPool, > _timeout: Option, > @@ -100,9 +99,8 @@ impl yew::Component for RemoteOverviewPanelComp { > mem_data: Rc::new(Series::new("", Vec::new())), > mem_total_data: Rc::new(Series::new("", Vec::new())), > rrd_time_frame: RRDTimeframe::load(), > - status: None, > + status: LoadResult::new(), > last_error: None, > - last_status_error: None, > async_pool: AsyncPool::new(), > _timeout: None, > _status_timeout: None, > @@ -160,15 +158,7 @@ impl yew::Component for RemoteOverviewPanelComp { > Err(err) => self.last_error = Some(err), > }, > Msg::StatusLoadFinished(res) => { > - match res { > - Ok(status) => { > - self.last_status_error = None; > - self.status = Some(status); > - } > - Err(err) => { > - self.last_status_error = Some(err); > - } > - } > + self.status.update(res); > let link = ctx.link().clone(); > self._status_timeout = Some(gloo_timers::callback::Timeout::new( > ctx.props().status_interval, > @@ -188,7 +178,7 @@ impl yew::Component for RemoteOverviewPanelComp { > let props = ctx.props(); > > if props.remote != old_props.remote { > - self.status = None; > + self.status = LoadResult::new(); > self.last_error = None; > self.time_data = Rc::new(Vec::new()); > self.cpu_data = Rc::new(Series::new("", Vec::new())); > @@ -205,9 +195,8 @@ impl yew::Component for RemoteOverviewPanelComp { > } > > fn view(&self, ctx: &yew::Context) -> yew::Html { > - let status_comp = node_info(self.status.as_ref().map(|s| s.into())); > + let status_comp = node_info(self.status.data.as_ref().map(|s| s.into())); > > - let loading = self.status.is_none() && self.last_status_error.is_none(); > let title: Html = Row::new() > .gap(2) > .class(AlignItems::Baseline) > @@ -221,12 +210,13 @@ impl yew::Component for RemoteOverviewPanelComp { > .with_child( > // FIXME: add some 'visible' or 'active' property to the progress > Progress::new() > - .value((!loading).then_some(0.0)) > - .style("opacity", (!loading).then_some("0")), > + .value(self.status.has_data().then_some(0.0)) > + .style("opacity", self.status.has_data().then_some("0")), > ) > .with_child(status_comp) > .with_optional_child( > - self.last_status_error > + self.status > + .error > .as_ref() > .map(|err| error_message(&err.to_string())), > ) > diff --git a/ui/src/pve/lxc/overview.rs b/ui/src/pve/lxc/overview.rs > index 41647275..5decbe6e 100644 > --- a/ui/src/pve/lxc/overview.rs > +++ b/ui/src/pve/lxc/overview.rs > @@ -19,6 +19,7 @@ use pdm_api_types::{resource::PveLxcResource, rrddata::LxcDataPoint}; > use pdm_client::types::{IsRunning, LxcStatus}; > > use crate::renderer::{separator, status_row}; > +use crate::LoadResult; > > #[derive(Clone, Debug, Properties, PartialEq)] > pub struct LxcOverviewPanel { > @@ -56,8 +57,7 @@ pub enum Msg { > } > > pub struct LxcanelComp { > - status: Option, > - last_status_error: Option, > + status: LoadResult, > last_rrd_error: Option, > _status_timeout: Option, > _rrd_timeout: Option, > @@ -104,12 +104,11 @@ impl yew::Component for LxcanelComp { > ctx.link() > .send_message_batch(vec![Msg::ReloadStatus, Msg::ReloadRrd]); > Self { > - status: None, > + status: LoadResult::new(), > _status_timeout: None, > _rrd_timeout: None, > _async_pool: AsyncPool::new(), > last_rrd_error: None, > - last_status_error: None, > > rrd_time_frame: RRDTimeframe::load(), > > @@ -144,15 +143,7 @@ impl yew::Component for LxcanelComp { > false > } > Msg::StatusResult(res) => { > - match res { > - Ok(status) => { > - self.last_status_error = None; > - self.status = Some(status); > - } > - Err(err) => { > - self.last_status_error = Some(err); > - } > - } > + self.status.update(res); > > self._status_timeout = Some(Timeout::new(props.status_interval, move || { > link.send_message(Msg::ReloadStatus) > @@ -211,8 +202,7 @@ impl yew::Component for LxcanelComp { > let props = ctx.props(); > > if props.remote != old_props.remote || props.info != old_props.info { > - self.status = None; > - self.last_status_error = None; > + self.status = LoadResult::new(); > self.last_rrd_error = None; > > self.time = Rc::new(Vec::new()); > @@ -236,7 +226,7 @@ impl yew::Component for LxcanelComp { > let props = ctx.props(); > > let mut status_comp = Column::new().gap(2).padding(4); > - let status = match &self.status { > + let status = match &self.status.data { > Some(status) => status, > None => &LxcStatus { > cpu: Some(props.info.cpu), > @@ -319,16 +309,14 @@ impl yew::Component for LxcanelComp { > HumanByte::from(status.maxdisk.unwrap_or_default() as u64).to_string(), > )); > > - let loading = self.status.is_none() && self.last_status_error.is_none(); > - > Panel::new() > .class(FlexFit) > .class(ColorScheme::Neutral) > .with_child( > // FIXME: add some 'visible' or 'active' property to the progress > Progress::new() > - .value((!loading).then_some(0.0)) > - .style("opacity", (!loading).then_some("0")), > + .value(self.status.has_data().then_some(0.0)) > + .style("opacity", self.status.has_data().then_some("0")), > ) > .with_child(status_comp) > .with_child(separator().padding_x(4)) > diff --git a/ui/src/pve/node/overview.rs b/ui/src/pve/node/overview.rs > index 1a98c004..c2f2958f 100644 > --- a/ui/src/pve/node/overview.rs > +++ b/ui/src/pve/node/overview.rs > @@ -17,7 +17,7 @@ use pwt::{ > use pdm_api_types::rrddata::NodeDataPoint; > use pdm_client::types::NodeStatus; > > -use crate::renderer::separator; > +use crate::{renderer::separator, LoadResult}; > > #[derive(Clone, Debug, Eq, PartialEq, Properties)] > pub struct NodeOverviewPanel { > @@ -62,12 +62,11 @@ pub struct NodeOverviewPanelComp { > load_data: Rc, > mem_data: Rc, > mem_total_data: Rc, > - status: Option, > + status: LoadResult, > > rrd_time_frame: RRDTimeframe, > > last_error: Option, > - last_status_error: Option, > > async_pool: AsyncPool, > _timeout: Option, > @@ -103,9 +102,8 @@ impl yew::Component for NodeOverviewPanelComp { > mem_data: Rc::new(Series::new("", Vec::new())), > mem_total_data: Rc::new(Series::new("", Vec::new())), > rrd_time_frame: RRDTimeframe::load(), > - status: None, > + status: LoadResult::new(), > last_error: None, > - last_status_error: None, > async_pool: AsyncPool::new(), > _timeout: None, > _status_timeout: None, > @@ -165,13 +163,7 @@ impl yew::Component for NodeOverviewPanelComp { > Err(err) => self.last_error = Some(err), > }, > Msg::StatusLoadFinished(res) => { > - match res { > - Ok(status) => { > - self.last_status_error = None; > - self.status = Some(status); > - } > - Err(err) => self.last_status_error = Some(err), > - } > + self.status.update(res); > let link = ctx.link().clone(); > self._status_timeout = Some(gloo_timers::callback::Timeout::new( > ctx.props().status_interval, > @@ -191,8 +183,7 @@ impl yew::Component for NodeOverviewPanelComp { > let props = ctx.props(); > > if props.remote != old_props.remote || props.node != old_props.node { > - self.status = None; > - self.last_status_error = None; > + self.status = LoadResult::new(); > self.last_error = None; > self.time_data = Rc::new(Vec::new()); > self.cpu_data = Rc::new(Series::new("", Vec::new())); > @@ -209,20 +200,20 @@ impl yew::Component for NodeOverviewPanelComp { > } > > fn view(&self, ctx: &yew::Context) -> yew::Html { > - let status_comp = node_info(self.status.as_ref().map(|s| s.into())); > - let loading = self.status.is_none() && self.last_status_error.is_none(); > + let status_comp = node_info(self.status.data.as_ref().map(|s| s.into())); > Container::new() > .class(FlexFit) > .class(ColorScheme::Neutral) > .with_child( > // FIXME: add some 'visible' or 'active' property to the progress > Progress::new() > - .value((!loading).then_some(0.0)) > - .style("opacity", (!loading).then_some("0")), > + .value(self.status.has_data().then_some(0.0)) > + .style("opacity", self.status.has_data().then_some("0")), > ) > .with_child(status_comp) > .with_optional_child( > - self.last_status_error > + self.status > + .error > .as_ref() > .map(|err| error_message(&err.to_string())), > ) > diff --git a/ui/src/pve/qemu/overview.rs b/ui/src/pve/qemu/overview.rs > index 3d715ebf..27b425b8 100644 > --- a/ui/src/pve/qemu/overview.rs > +++ b/ui/src/pve/qemu/overview.rs > @@ -16,6 +16,7 @@ use pdm_api_types::{resource::PveQemuResource, rrddata::QemuDataPoint}; > use pdm_client::types::{IsRunning, QemuStatus}; > > use crate::renderer::{separator, status_row}; > +use crate::LoadResult; > > #[derive(Clone, Debug, Properties, PartialEq)] > pub struct QemuOverviewPanel { > @@ -53,8 +54,7 @@ pub enum Msg { > } > > pub struct QemuOverviewPanelComp { > - status: Option, > - last_status_error: Option, > + status: LoadResult, > last_rrd_error: Option, > _status_timeout: Option, > _rrd_timeout: Option, > @@ -101,12 +101,11 @@ impl yew::Component for QemuOverviewPanelComp { > ctx.link() > .send_message_batch(vec![Msg::ReloadStatus, Msg::ReloadRrd]); > Self { > - status: None, > + status: LoadResult::new(), > _status_timeout: None, > _rrd_timeout: None, > _async_pool: AsyncPool::new(), > last_rrd_error: None, > - last_status_error: None, > > rrd_time_frame: RRDTimeframe::load(), > > @@ -141,16 +140,7 @@ impl yew::Component for QemuOverviewPanelComp { > false > } > Msg::StatusResult(res) => { > - match res { > - Ok(status) => { > - self.last_status_error = None; > - self.status = Some(status); > - } > - Err(err) => { > - self.last_status_error = Some(err); > - } > - } > - > + self.status.update(res); > self._status_timeout = Some(Timeout::new(props.status_interval, move || { > link.send_message(Msg::ReloadStatus) > })); > @@ -210,8 +200,7 @@ impl yew::Component for QemuOverviewPanelComp { > let props = ctx.props(); > > if props.remote != old_props.remote || props.info != old_props.info { > - self.status = None; > - self.last_status_error = None; > + self.status = LoadResult::new(); > self.last_rrd_error = None; > > self.time = Rc::new(Vec::new()); > @@ -235,7 +224,7 @@ impl yew::Component for QemuOverviewPanelComp { > let props = ctx.props(); > let mut status_comp = Column::new().gap(2).padding(4); > > - let status = match &self.status { > + let status = match &self.status.data { > Some(status) => status, > None => &QemuStatus { > agent: None, > @@ -329,15 +318,14 @@ impl yew::Component for QemuOverviewPanelComp { > HumanByte::from(status.maxdisk.unwrap_or_default() as u64).to_string(), > )); > > - let loading = self.status.is_none() && self.last_status_error.is_none(); > Panel::new() > .class(pwt::css::FlexFit) > .class(pwt::css::ColorScheme::Neutral) > .with_child( > // FIXME: add some 'visible' or 'active' property to the progress > Progress::new() > - .value((!loading).then_some(0.0)) > - .style("opacity", (!loading).then_some("0")), > + .value(self.status.has_data().then_some(0.0)) > + .style("opacity", self.status.has_data().then_some("0")), > ) > .with_child(status_comp) > .with_child(separator().padding_x(4)) > diff --git a/ui/src/pve/storage.rs b/ui/src/pve/storage.rs > index 93a2fa29..9730d751 100644 > --- a/ui/src/pve/storage.rs > +++ b/ui/src/pve/storage.rs > @@ -22,6 +22,7 @@ use pdm_client::types::PveStorageStatus; > use crate::{ > pve::utils::{render_content_type, render_storage_type}, > renderer::{separator, status_row_right_icon}, > + LoadResult, > }; > > #[derive(Clone, Debug, Properties)] > @@ -74,8 +75,7 @@ pub enum Msg { > } > > pub struct StoragePanelComp { > - status: Option, > - last_status_error: Option, > + status: LoadResult, > last_rrd_error: Option, > > /// internal guard for the periodic timeout callback to update status > @@ -131,12 +131,11 @@ impl yew::Component for StoragePanelComp { > ctx.link() > .send_message_batch(vec![Msg::ReloadStatus, Msg::ReloadRrd]); > Self { > - status: None, > + status: LoadResult::new(), > status_update_timeout_guard: None, > rrd_update_timeout_guard: None, > _async_pool: AsyncPool::new(), > last_rrd_error: None, > - last_status_error: None, > > rrd_time_frame: RRDTimeframe::load(), > > @@ -167,16 +166,7 @@ impl yew::Component for StoragePanelComp { > false > } > Msg::StatusResult(res) => { > - match res { > - Ok(status) => { > - self.last_status_error = None; > - self.status = Some(status); > - } > - Err(err) => { > - self.last_status_error = Some(err); > - } > - } > - > + self.status.update(res); > self.status_update_timeout_guard = > Some(Timeout::new(props.status_interval, move || { > link.send_message(Msg::ReloadStatus) > @@ -220,8 +210,7 @@ impl yew::Component for StoragePanelComp { > let props = ctx.props(); > > if props.remote != old_props.remote || props.info != old_props.info { > - self.status = None; > - self.last_status_error = None; > + self.status = LoadResult::new(); > self.last_rrd_error = None; > > self.time = Rc::new(Vec::new()); > @@ -246,7 +235,7 @@ impl yew::Component for StoragePanelComp { > .into(); > > let mut status_comp = Column::new().gap(2).padding(4); > - let status = match &self.status { > + let status = match &self.status.data { > Some(status) => status, > None => &PveStorageStatus { > active: None, > @@ -305,8 +294,6 @@ impl yew::Component for StoragePanelComp { > .value(disk_usage as f32), > ); > > - let loading = self.status.is_none() && self.last_status_error.is_none(); > - > Panel::new() > .class(FlexFit) > .title(title) > @@ -314,8 +301,8 @@ impl yew::Component for StoragePanelComp { > .with_child( > // FIXME: add some 'visible' or 'active' property to the progress > Progress::new() > - .value((!loading).then_some(0.0)) > - .style("opacity", (!loading).then_some("0")), > + .value(self.status.has_data().then_some(0.0)) > + .style("opacity", self.status.has_data().then_some("0")), > ) > .with_child(status_comp) > .with_child(separator().padding_x(4)) _______________________________________________ pdm-devel mailing list pdm-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel