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 7E55B1FF13F for ; Thu, 09 Apr 2026 20:26:49 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id AB775C801; Thu, 9 Apr 2026 20:27:32 +0200 (CEST) Message-ID: <56402a07-1b4d-4075-9fe6-c20b43263e54@proxmox.com> Date: Thu, 9 Apr 2026 20:26:58 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Beta Subject: Re: [PATCH datacenter-manager v2] Add notifications backend To: Lukas Wagner , Shannon Sterz , Arthur Bied-Charreton , pdm-devel@lists.proxmox.com References: <20260409045819.19858-1-a.bied-charreton@proxmox.com> Content-Language: en-US, de-DE From: Thomas Lamprecht In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1775759150190 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.001 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 Message-ID-Hash: L53XC3C6CU6ZFH2IEHHIPKJHIY4R3N22 X-Message-ID-Hash: L53XC3C6CU6ZFH2IEHHIPKJHIY4R3N22 X-MailFrom: t.lamprecht@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 Datacenter Manager development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On 09/04/2026 14:07, Lukas Wagner wrote: > This was coordinated with me, I told him that it would be okay to send > this part early. He can either build on top of this series and include > more patches (e.g. adding the GUI, notification events, docs, etc.) in > future versions, or if it makes sense we can also apply certain parts > early. I'd strongly recommend to generally send along (basic) integration of such factoring out work, as else it's basically impossible to review and see that this can work out without the reviewer having to do the work themselves. This doesn't have to be a fully fledged out thing for an initial review, but here some basic notification wouldn't be that much (and has to be done anyway, as how else was this tested?) btw. I (and probably others) would also appreciate if such discussion happens in some shared channel (e.g. zulip). On 09/04/2026 14:07, Lukas Wagner wrote: >> this would probably also benefit from checking the config digest. >> > Same here, I'm not really sure what the benefit would be here? > > IMO there are a couple cases to consider for concurrent modifications: > - somebody modified entity A, we delete B -> should be fine > - somebody modified entity A, we delete A -> does not matter, we want > to delete it anyways but we did so with the old config values in mind. The somebody might have added changed a comment from "test" to "production, don't delete" The point of digest checks are to ensure an action is executed with the same information the client saw when making the decision. > - we deleted A, somebody else modifies A -> update fails anyways due > to the wrong config digest or the entity missing already > - both try to delete A -> fails for one of both due to the already > deleted entity (HTTP 404) > > Did I miss something? Another one would be: Somebody deleted A, somebody creates A, we now delete (another!) A. Config digest checks are a common thing in most of our stacks for a reason, they certainly are not the most elaborate/best tool, but they are simple and protect against any decisions that got wrong due to out-of-date information. I'd be fine with something better, but ideally it's some slight variation of the current system, like e.g. reducing the digest assertion-scope from the whole config to an config entry (not always useful if there are cross-references in the config), bigger change can be OK, but naturally needs much more justification and ensuring that it can be used more than once (ideally everywhere).