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 60E1E1FF16B for ; Fri, 7 Nov 2025 09:37:34 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id DDD819E15; Fri, 7 Nov 2025 09:38:15 +0100 (CET) Mime-Version: 1.0 Date: Fri, 07 Nov 2025 09:37:32 +0100 Message-Id: To: "Lukas Wagner" X-Mailer: aerc 0.20.0 References: <20251106124334.189955-1-s.sterz@proxmox.com> <20251106124334.189955-6-s.sterz@proxmox.com> In-Reply-To: From: "Shannon Sterz" X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1762504633113 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.061 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 RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. 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] [PATCH datacenter-manager 2/3] ui: make NodeStatusPanel available as a widget 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 , Thomas Lamprecht Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pdm-devel-bounces@lists.proxmox.com Sender: "pdm-devel" On Fri Nov 7, 2025 at 8:31 AM CET, Lukas Wagner wrote: > On Thu Nov 6, 2025 at 10:10 PM CET, Thomas Lamprecht wrote: >> Am 06.11.25 um 13:43 schrieb Shannon Sterz: >>> Signed-off-by: Shannon Sterz >>> --- >>> ui/src/administration/mod.rs | 4 +++- >>> ui/src/dashboard/types.rs | 1 + >>> ui/src/dashboard/view.rs | 15 ++++++++++++++- >>> 3 files changed, 18 insertions(+), 2 deletions(-) >>> >>> diff --git a/ui/src/administration/mod.rs b/ui/src/administration/mod.rs >>> index a9f7ac6..72a4ffa 100644 >>> --- a/ui/src/administration/mod.rs >>> +++ b/ui/src/administration/mod.rs >>> @@ -16,7 +16,9 @@ use pwt_macros::builder; >>> //mod services; >>> //pub use services::Services; >>> >>> -use proxmox_yew_comp::{AptPackageManager, AptRepositories, ExistingProduct, Syslog, Tasks}; >>> +use proxmox_yew_comp::{ >>> + AptPackageManager, AptRepositories, ExistingProduct, NodeStatusPanel, Syslog, Tasks, >>> +}; >>> >>> #[derive(Clone, PartialEq, Properties)] >>> #[builder] >>> diff --git a/ui/src/dashboard/types.rs b/ui/src/dashboard/types.rs >>> index c79c38a..ce4bbb7 100644 >>> --- a/ui/src/dashboard/types.rs >>> +++ b/ui/src/dashboard/types.rs >>> @@ -61,6 +61,7 @@ pub enum WidgetType { >>> TaskSummary { >>> grouping: TaskSummaryGrouping, >>> }, >>> + NodeStatus, >>> } >>> >>> #[derive(Serialize, Deserialize, PartialEq, Clone, Copy)] >>> diff --git a/ui/src/dashboard/view.rs b/ui/src/dashboard/view.rs >>> index c781d99..b5cd722 100644 >>> --- a/ui/src/dashboard/view.rs >>> +++ b/ui/src/dashboard/view.rs >>> @@ -3,6 +3,7 @@ use std::rc::Rc; >>> use anyhow::Error; >>> use futures::join; >>> use js_sys::Date; >>> +use proxmox_yew_comp::NodeStatusPanel; >>> use serde_json::json; >>> use yew::virtual_dom::{VComp, VNode}; >>> >>> @@ -123,6 +124,11 @@ fn render_widget( >>> let (hours, since) = get_task_options(refresh_config.task_last_hours); >>> create_task_summary_panel(statistics, remotes, hours, since) >>> } >>> + WidgetType::NodeStatus => { >>> + return NodeStatusPanel::new() >>> + .status_base_url("/nodes/localhost/status") >>> + .into() >>> + } >>> }; >>> >> >> While it might be OK to have as widget for remote node status in the dashboard, I >> would not add the one from PDM here, that's IMO confusing, especially if it's >> nowhere stated that it is in fact the one from PDM itself, given that all other >> widgets show the status of remote resources. >> I.e., the PBS dashboard is for the PBS instance itself, the PDM one is mainly for >> the remote resources, they need to be handled a bit differently when deciding what >> to add where, or at least ensure that it's clear that some info is from PDM itself. >> >> But rather lets add a new tab to the Administration panel, there we can then also >> add metrics graphs (memory, cpu, ...) for the local PDM itself in the future. >> Could be still nice to implement a PDM specific overview for the dashboard, but >> that should be probably something new with the specifics of PDM in mind, and can >> be done separately. >> Reboot/shutdown could then go in the toolbar there. >> > > +1 > > I quickly tried this patch series out yesterday before going home, my > findings/thoughts were > > - As you mentioned, the fact that this widget is about the PDM host > itself is not super clear (no mention of it in the title, and all the > other widgets are about remotes) > > - The widget appears a bit too dense/crowded, at least compared to the > other widgets that we already have. I think this mostly due to the > three buttons, as well as the long kernel version string, on my PDM > dev host this is > > Linux 6.14.8-2-pve #1 SMP PREEMPT_DYNAMIC PMX 6.14.8-2 (2025-07-22T10:04Z) > > which appears to be quite a bit longer than what we show in PVE/PBS: > > Linux 6.14.11-4-pve (2025-10-10T08:04Z) just a small note, this is how seem to display this information throughout pdm at least, so i'd keep that for now. > - Having the potentially disruptive Reboot/Shutdown buttons so prominent > in the dashboard can be a bit 'dangerous', I think - especially since > it's not 100% clear at the moment that this is the PDM host itself. > > > I fully agree with you proposal, I think putting this into a new tab in > the Administration panel definitely makes a lot of sense. > >> It also might be more useful to not show the "Show Fingerprint" button by default. >> Adding that in this card was a bit of a stop-gap for PBS on it's own that I >> implemented only because knowing the fingerprint is quite often a must-have for >> being able to add a PBS as storage to a PVE cluster. As we do not can add a PDM >> somewhere (in our projects) at all for now, there is really not such a need there. >> >> FWIW, for PBS we have reduced the need for having this in the node status directly >> too, as there we now got the "connection info" button/window available on the >> Datastore summary page, which also includes the full datastore repository string, >> and is thus more convenient than the dashboard one. >> That's also why I'd not show this button by default at all, at least in the newer >> Yew UI. alright i'll make the fingerprint opt-in and revert to the tab based approach from v1 then. _______________________________________________ pdm-devel mailing list pdm-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel