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 4BE5A1FF13F for ; Thu, 26 Feb 2026 11:18:22 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 8AF4B1B75B; Thu, 26 Feb 2026 11:19:16 +0100 (CET) Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8 Date: Thu, 26 Feb 2026 11:18:41 +0100 Message-Id: From: "Daniel Kral" To: "Dominik Rusovac" , Subject: Re: [PATCH proxmox] resource-scheduling: topsis: add context to 'unwraps' X-Mailer: aerc 0.21.0-38-g7088c3642f2c-dirty References: <20260225130639.282334-1-d.rusovac@proxmox.com> In-Reply-To: X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1772101103824 X-SPAM-LEVEL: Spam detection results: 0 AWL -1.038 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.618 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.734 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.78 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 Message-ID-Hash: NIWLL523LHA2PAKYMG22SSPQIY2WF74D X-Message-ID-Hash: NIWLL523LHA2PAKYMG22SSPQIY2WF74D X-MailFrom: d.kral@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox VE development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On Thu Feb 26, 2026 at 10:28 AM CET, Dominik Rusovac wrote: > I agree, unwrapping the option is indeed questionable. Yet these unwraps > seemed good enough and I didn't want to touch the error handling itself, > as it would entail more changes. Atm, we have no designated Error types > to map to and imo just bail!-ing on a Result in this place adds no > reasonable value. Consistently propagating errors from start to end > would be more appropriate, I suppose. > > For now, I just added some context as to what must have gone wrong in > this very situation, because it changes just a tiny bit of code, while > adding reasonable information.=20 > > I could send a v2 that touches error handling in general, though. Currently, the only user of score_alternatives(...) is score_nodes_to_start_service(...), where it's enough that nodes is an empty slice to cause a panic. This isn't a problem now because the function is unlikely to be called as there's always at least one node available. If we're going to use these for the load balancer, it could be way easier to run into this, e.g. there is only one service restricted to a single node, i.e., no migration candidates. Of course we could catch this early, but returning a generic Error here and then handling that in score_alternatives(...) to return an empty vec would be an improvement. Having error types would be plus here of course, but needs some consideration that it doesn't break building packages that use this crate.