From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id D9D951FF17E for ; Thu, 16 Oct 2025 11:09:07 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 61243D8C6; Thu, 16 Oct 2025 11:09:28 +0200 (CEST) Mime-Version: 1.0 Date: Thu, 16 Oct 2025 11:09:24 +0200 Message-Id: Cc: "pdm-devel" From: "Lukas Wagner" To: "Proxmox Datacenter Manager development discussion" X-Mailer: aerc 0.20.1-0-g2ecb8770224a References: <20251013083806.1068917-1-d.csapak@proxmox.com> <20251013083806.1068917-4-d.csapak@proxmox.com> In-Reply-To: <20251013083806.1068917-4-d.csapak@proxmox.com> X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1760605761809 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.027 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 3/3] api/ui: pve: get cluster resources from cache if remote is unreachable 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 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pdm-devel-bounces@lists.proxmox.com Sender: "pdm-devel" On Mon Oct 13, 2025 at 10:33 AM CEST, Dominik Csapak wrote: > so that the last cached data can be shown instead of an error. > This now means that we'll always use the full api call here and filter > on the PDM side to get a fresh version in the cache. > > The 'kind' parameter is not sent to the pve backend anymore but it's > filtered on the pdm side. > > To show a notice in the ui that it was loaded from the cache, we need to > add a marker property in the result, and need to extract that in the > pdm-client so the gui has access to that. > > Signed-off-by: Dominik Csapak > --- > sending as rfc because > * it changes how we handle the 'kind' parameter > * we may want to opt in to that behaviour? > * not sure if a result side channel is the best way to convey the > 'from-cache' state. I think think this should be part of the API response type, the 'attribs' mechanic feels a bit too magic for me. So instead of return a Vec, maybe a PveResourceList #[api] struct PveResourceList { resources: Vec, cached: bool, cache_timestamp: ... // Could be useful as well } Recently I've started to avoid returning a bare Vec<...> from API endpoints in general, basically exactly for cases like these, where we want to augment the endpoint with additional data later on. > > lib/pdm-client/src/lib.rs | 10 ++++++-- > server/src/api/pve/mod.rs | 48 ++++++++++++++++++++++++------------- > server/src/api/resources.rs | 20 ++++++++++++++++ > ui/src/pve/mod.rs | 14 +++++++---- > 4 files changed, 70 insertions(+), 22 deletions(-) > > diff --git a/lib/pdm-client/src/lib.rs b/lib/pdm-client/src/lib.rs > index 2f36fab1..3ebb1c35 100644 > --- a/lib/pdm-client/src/lib.rs > +++ b/lib/pdm-client/src/lib.rs > @@ -408,11 +408,17 @@ impl PdmClient { > &self, > remote: &str, > kind: Option, > - ) -> Result, Error> { > + ) -> Result<(bool, Vec), Error> { > let query = ApiPathBuilder::new(format!("/api2/extjs/pve/remotes/{remote}/resources")) > .maybe_arg("kind", &kind) > .build(); > - Ok(self.0.get(&query).await?.expect_json()?.data) > + let api_response_data = self.0.get(&query).await?.expect_json()?; > + let from_cache = api_response_data > + .attribs > + .get("from-cache") > + .and_then(|val| val.as_bool()) > + .unwrap_or(false); > + Ok((from_cache, api_response_data.data)) > } > > pub async fn pve_cluster_status( > diff --git a/server/src/api/pve/mod.rs b/server/src/api/pve/mod.rs > index 0de82323..c16d6de5 100644 > --- a/server/src/api/pve/mod.rs > +++ b/server/src/api/pve/mod.rs > @@ -16,19 +16,18 @@ use proxmox_sortable_macro::sortable; > use pdm_api_types::remotes::{ > NodeUrl, Remote, RemoteListEntry, RemoteType, TlsProbeOutcome, REMOTE_ID_SCHEMA, > }; > -use pdm_api_types::resource::PveResource; > +use pdm_api_types::resource::{PveResource, Resource}; > use pdm_api_types::{ > Authid, RemoteUpid, HOST_OPTIONAL_PORT_FORMAT, PRIV_RESOURCE_AUDIT, PRIV_RESOURCE_DELETE, > PRIV_SYS_MODIFY, > }; > > use pve_api_types::ClusterNodeStatus; > +use pve_api_types::ClusterResourceKind; > use pve_api_types::ListRealm; > use pve_api_types::PveUpid; > -use pve_api_types::{ClusterResourceKind, ClusterResourceType}; > - > -use super::resources::{map_pve_lxc, map_pve_node, map_pve_qemu, map_pve_storage}; > > +use crate::api::resources::get_updated_resources_or_cache; > use crate::connection::PveClient; > use crate::connection::{self, probe_tls_connection}; > use crate::remote_tasks; > @@ -202,13 +201,29 @@ pub async fn cluster_resources( > http_bail!(UNAUTHORIZED, "user has no access to resource list"); > } > > - let cluster_resources = connect_to_remote(&remotes, &remote)? > - .cluster_resources(kind) > - .await? > + let remote = get_remote(&remotes, &remote)?; > + > + let (from_cache, cluster_resources) = get_updated_resources_or_cache(remote).await?; > + > + rpcenv.result_attrib_mut()["from-cache"] = from_cache.into(); > + > + let cluster_resources = cluster_resources > .into_iter() > - .filter_map(|r| map_pve_resource(&remote, r)); > + .filter(|res| match kind { > + Some(kind) => matches!( > + (kind, res), > + (ClusterResourceKind::Vm, Resource::PveQemu(_)) > + | (ClusterResourceKind::Vm, Resource::PveLxc(_)) > + | (ClusterResourceKind::Storage, Resource::PveStorage(_)) > + | (ClusterResourceKind::Node, Resource::PveNode(_)) > + | (ClusterResourceKind::Sdn, Resource::PveSdn(_)) > + ), > + None => true, > + }) > + .filter_map(map_pve_resource) > + .collect(); > > - Ok(cluster_resources.collect()) > + Ok(cluster_resources) > } > > #[api( > @@ -245,13 +260,14 @@ pub async fn cluster_status( > Ok(status) > } > > -fn map_pve_resource(remote: &str, resource: pve_api_types::ClusterResource) -> Option { > - match resource.ty { > - ClusterResourceType::Node => map_pve_node(remote, resource).map(PveResource::Node), > - ClusterResourceType::Lxc => map_pve_lxc(remote, resource).map(PveResource::Lxc), > - ClusterResourceType::Qemu => map_pve_qemu(remote, resource).map(PveResource::Qemu), > - ClusterResourceType::Storage => map_pve_storage(remote, resource).map(PveResource::Storage), > - _ => None, > +fn map_pve_resource(resource: Resource) -> Option { > + match resource { > + Resource::PveStorage(storage) => Some(PveResource::Storage(storage)), > + Resource::PveQemu(qemu) => Some(PveResource::Qemu(qemu)), > + Resource::PveLxc(lxc) => Some(PveResource::Lxc(lxc)), > + Resource::PveNode(node) => Some(PveResource::Node(node)), > + Resource::PveSdn(sdn) => Some(PveResource::Sdn(sdn)), > + Resource::PbsNode(_) | Resource::PbsDatastore(_) => None, > } > } > > diff --git a/server/src/api/resources.rs b/server/src/api/resources.rs > index 5b7ac524..5b876998 100644 > --- a/server/src/api/resources.rs > +++ b/server/src/api/resources.rs > @@ -645,6 +645,26 @@ async fn get_resources_for_remote(remote: &Remote, max_age: u64) -> Result .await > } > > +/// Tries to update the resources and returns data from the cache if it was not > +/// possible to reach the remote. This is indacted in the return value when > +/// `(true, data)` is returned. > +pub async fn get_updated_resources_or_cache( > + remote: &Remote, > +) -> Result<(bool, Vec), Error> { > + match CACHE > + .get_updated_value(&remote.id, 0, |_| async move { > + fetch_remote_resource(remote).await > + }) > + .await > + { > + Ok(data) => Ok((false, data)), > + Err(err) => match CACHE.get_cached_value(&remote.id, i64::MAX) { > + Some(data) => Ok((true, data)), > + None => Err(err.context("cache was empty and could not update")), > + }, > + } > +} > + > /// Read cached resource data from the cache > pub fn get_cached_resources(remote: &str, max_age: u64) -> Option> { > CACHE.get_cached_value(remote, max_age as i64) > diff --git a/ui/src/pve/mod.rs b/ui/src/pve/mod.rs > index 058424a9..2cdc99f7 100644 > --- a/ui/src/pve/mod.rs > +++ b/ui/src/pve/mod.rs > @@ -11,7 +11,7 @@ use yew::{ > use pwt::{ > css::AlignItems, > state::NavigationContainer, > - widget::{Button, Container, Fa}, > + widget::{error_message, Button, Container, Fa}, > }; > use pwt::{ > css::FlexFit, > @@ -122,13 +122,14 @@ impl GuestInfo { > > pub enum Msg { > SelectedView(tree::PveTreeNode), > - ResourcesList(Result, Error>), > + ResourcesList(Result<(bool, Vec), Error>), > } > > pub struct PveRemoteComp { > view: tree::PveTreeNode, > resources: Rc>, > last_error: Option, > + from_cache: bool, > } > > impl LoadableComponent for PveRemoteComp { > @@ -142,6 +143,7 @@ impl LoadableComponent for PveRemoteComp { > view: PveTreeNode::Root, > resources: Rc::new(Vec::new()), > last_error: None, > + from_cache: false, > } > } > > @@ -151,8 +153,9 @@ impl LoadableComponent for PveRemoteComp { > self.view = node; > } > Msg::ResourcesList(res) => match res { > - Ok(res) => { > + Ok((from_cache, res)) => { > self.last_error = None; > + self.from_cache = from_cache; > self.resources = Rc::new(res); > } > Err(err) => { > @@ -245,7 +248,10 @@ impl LoadableComponent for PveRemoteComp { > let link = link.clone(); > move |_| link.send_reload() > }, > - )), > + )) > + .with_optional_child(self.from_cache.then_some(error_message(&tr!( > + "Could not reach remote, data shown from cache." > + )))), > ), > ) > .with_child( _______________________________________________ pdm-devel mailing list pdm-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel