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 011A21FF15E for ; Mon, 10 Nov 2025 16:29:30 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id B9E081A3D4; Mon, 10 Nov 2025 16:30:14 +0100 (CET) Message-ID: <98efbc2c-378a-4c35-b461-a5549fdba17f@proxmox.com> Date: Mon, 10 Nov 2025 16:29:40 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: Proxmox Datacenter Manager development discussion , Lukas Wagner References: <20251105163546.450094-1-h.laimer@proxmox.com> <20251105163546.450094-13-h.laimer@proxmox.com> Content-Language: en-US From: Hannes Laimer In-Reply-To: X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1762788558320 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.046 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] [PATCH proxmox-datacenter-manager v2 4/4] ui: add firewall status tree 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-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: pdm-devel-bounces@lists.proxmox.com Sender: "pdm-devel" On 11/7/25 13:27, Lukas Wagner wrote: > Looks good from what I can tell (not a super deep review though), some > notes inline. > > There is some small bug that I've found, when you use the << button to > collapse the tree and then expand it again, it is slightly smaller than > before, noticable from the "x out of y rules enabled" text not fitting > the view any more. > hmm, I couldn't reproduce this here. For v3 I'll replace the .flex(1) with a fixed size, that should eliminate any potential _wonkiness_ > Also we need to find some way to handle unavailable remotes more > gracefully. Right now, a single unavailable remote can block the entire > loading progress until that one eventually times out; this is definitely > not great UX-wise. But I think this can also be improved in a future > patch series, IMO no need to block this until then. > yes. they are loaded in parallel, so max wait time is ~whatever our timeout is. Maybe a lower timeout could make sense? (I *think* we have that for remote resources...) > On Wed Nov 5, 2025 at 5:35 PM CET, Hannes Laimer wrote: >> + let mut remote_handle = root.append(remote_entry); >> + remote_handle.set_expanded(cluster_is_enabled); >> + >> + for node_status in remote_status.nodes { >> + let node_name = node_status.node.clone(); >> + let node_firewall_status = node_status.status; >> + >> + let node_entry = TreeEntry::Node(NodeEntry { >> + remote: remote_name.clone(), >> + name: node_name.clone(), >> + status: node_firewall_status, >> + masked: !cluster_is_enabled, >> + }); >> + >> + let mut node_handle = remote_handle.append(node_entry); >> + node_handle.set_expanded(!node_status.guests.is_empty()); >> + >> + for guest in node_status.guests { >> + let guest_entry = GuestEntry::new( >> + guest.clone(), >> + node_name.clone(), >> + remote_name.clone(), >> + !cluster_is_enabled, >> + ); >> + >> + let tree_entry = match guest.kind { >> + GuestKind::Lxc => TreeEntry::Guest(guest_entry, GuestKind::Lxc), >> + GuestKind::Qemu => TreeEntry::Guest(guest_entry, GuestKind::Qemu), >> + }; > > This can just be: > > let tree_entry = TreeEntry::Guest(guest_entry, guest.kind); > >> + >> + node_handle.append(tree_entry); >> + } >> + } >> +} >> + >> +fn sort_entries(a: &TreeEntry, b: &TreeEntry) -> Ordering { >> + let rank_a = a.sort_rank(); >> + let rank_b = b.sort_rank(); >> + match rank_a.cmp(&rank_b) { >> + Ordering::Equal => a.name().cmp(&b.name()), >> + other => other, >> + } >> +} >> + >> +pub enum Msg { >> + DataLoaded { >> + generation: usize, >> + data: Vec, >> + }, >> + RemoteListChanged, >> + Reload, >> + FilterChanged(String), >> + ScopeChanged(Scope), >> + RemotesLoaded(Vec), >> + NodesLoaded { >> + generation: usize, >> + nodes: Vec, >> + }, >> + SelectionChanged(Option), >> + ToggleTreePanel, >> + Error(FirewallError), >> + NoOp, >> +} >> + > > [...] > >> + >> +fn create_remote_combobox( >> + ctx: &LoadableComponentContext, >> + available_remotes: &[String], >> + options_loading: bool, >> + current_scope: &Scope, >> +) -> Html { >> + if options_loading { >> + return Combobox::new() >> + .items(Rc::new(vec![])) >> + .placeholder(tr!("Loading...")) >> + .disabled(true) >> + .key("remote-combobox-loading") >> + .on_change(ctx.link().callback(|_: String| Msg::NoOp)) >> + .into(); >> + } >> + >> + let items: Vec = available_remotes >> + .iter() >> + .map(|remote| yew::AttrValue::from(remote.clone())) >> + .collect(); >> + >> + let current_value = current_scope >> + .remote_name() >> + .map(|s| yew::AttrValue::from(s.to_string())); >> + >> + Combobox::new() >> + .items(Rc::new(items)) >> + .default(current_value) >> + .placeholder(tr!("All remotes")) > > This combobox also shows PBS remotes, I guess they need to be filtered > out somewhere along the way. > >> + .disabled(false) >> + .key("remote-combobox") >> + .on_change(ctx.link().callback(move |value: String| { >> + if value.is_empty() { >> + Msg::ScopeChanged(Scope::All) >> + } else { >> + Msg::ScopeChanged(Scope::Remote { name: value }) >> + } >> + })) >> + .into() >> +} >> + > > > > _______________________________________________ > pdm-devel mailing list > pdm-devel@lists.proxmox.com > https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel > > _______________________________________________ pdm-devel mailing list pdm-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel