all lists on lists.proxmox.com
 help / color / mirror / Atom feed
From: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
To: Thomas Lamprecht <t.lamprecht@proxmox.com>
Cc: pdm-devel@lists.proxmox.com
Subject: Re: [PATCH datacenter-manager v2] Add notifications backend
Date: Fri, 10 Apr 2026 14:00:21 +0200	[thread overview]
Message-ID: <zemlumbv4zg44jfth2fda7h7ajn47xltc7k4ns6mtgvcaxu2p7@apwctc4gbne5> (raw)
In-Reply-To: <56402a07-1b4d-4075-9fe6-c20b43263e54@proxmox.com>

On Thu, Apr 09, 2026 at 08:26:58PM +0200, Thomas Lamprecht wrote:
> 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?)
> 
I tested it with curl scripts, I agree that this is not ideal for the
reviewing process. Will send v2 along with integrated notifications.

> btw. I (and probably others) would also appreciate if such discussion
> happens in some shared channel (e.g. zulip).
> 
Noted.
> 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).

Thanks a lot for the context, I will integrate whole config digest 
checks in the handlers in v2.




      reply	other threads:[~2026-04-10 11:59 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-04-09  4:57 Arthur Bied-Charreton
2026-04-09  9:20 ` Shannon Sterz
2026-04-09 10:18   ` Arthur Bied-Charreton
2026-04-09 11:13     ` Shannon Sterz
2026-04-09 12:07   ` Lukas Wagner
2026-04-09 12:39     ` Shannon Sterz
2026-04-09 18:26     ` Thomas Lamprecht
2026-04-10 12:00       ` Arthur Bied-Charreton [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=zemlumbv4zg44jfth2fda7h7ajn47xltc7k4ns6mtgvcaxu2p7@apwctc4gbne5 \
    --to=a.bied-charreton@proxmox.com \
    --cc=pdm-devel@lists.proxmox.com \
    --cc=t.lamprecht@proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal