From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <pbs-devel-bounces@lists.proxmox.com>
Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9])
by lore.proxmox.com (Postfix) with ESMTPS id 6737D1FF191
for <inbox@lore.proxmox.com>; Mon, 2 Jun 2025 16:36:03 +0200 (CEST)
Received: from firstgate.proxmox.com (localhost [127.0.0.1])
by firstgate.proxmox.com (Proxmox) with ESMTP id A8BEA36E18;
Mon, 2 Jun 2025 16:36:21 +0200 (CEST)
Message-ID: <aae080ae-38ae-49d3-8a11-3376b57c0092@proxmox.com>
Date: Mon, 2 Jun 2025 16:35:47 +0200
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
From: Lukas Wagner <l.wagner@proxmox.com>
To: pbs-devel@lists.proxmox.com
References: <20250521142309.264719-1-l.wagner@proxmox.com>
Content-Language: de-AT, en-US
In-Reply-To: <20250521142309.264719-1-l.wagner@proxmox.com>
X-SPAM-LEVEL: Spam detection results: 0
AWL 0.020 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: [pbs-devel] [PATCH proxmox{, -widget-toolkit,
-backup} 0/4] notifications: add support for nested matchers
X-BeenThere: pbs-devel@lists.proxmox.com
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Proxmox Backup Server development discussion
<pbs-devel.lists.proxmox.com>
List-Unsubscribe: <https://lists.proxmox.com/cgi-bin/mailman/options/pbs-devel>,
<mailto:pbs-devel-request@lists.proxmox.com?subject=unsubscribe>
List-Archive: <http://lists.proxmox.com/pipermail/pbs-devel/>
List-Post: <mailto:pbs-devel@lists.proxmox.com>
List-Help: <mailto:pbs-devel-request@lists.proxmox.com?subject=help>
List-Subscribe: <https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel>,
<mailto:pbs-devel-request@lists.proxmox.com?subject=subscribe>
Reply-To: Proxmox Backup Server development discussion
<pbs-devel@lists.proxmox.com>
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Errors-To: pbs-devel-bounces@lists.proxmox.com
Sender: "pbs-devel" <pbs-devel-bounces@lists.proxmox.com>
On 2025-05-21 16:23, Lukas Wagner wrote:
> This series adds support for nested notification matchers. A new match rule is
> added, 'eval-matcher', which allows to evaluate other matchers and use their
> result in the current matcher.
>
> Any matcher that shall be used as a nested matcher must have the (new) `nested`
> flag set to true. These matchers are only evaluated if they are referenced
> by another matcher. Any target configured for a nested matcher is ignored,
> only the 'top-level'/'outermost' matcher decides which targets to notify.
>
> This patch series includes:
> - patches for proxmox-notify, which add the new config entries for matchers,
> API methods and nested matcher evaluation
> - patches for proxmox-backup, which contain documentation for the new
> feature
> - patches for proxmox-widget-toolkit, containing the GUI changes needed
> for this feature.
>
> Small config example:
>
> matcher: top-level
> target mail-to-root
> mode any
> eval-matcher a
> eval-matcher b
>
> matcher: a
> nested true
> mode all
> match-field exact:datastore=store
> match-field exact:type=gc
> match-severity error
>
> matcher: b
> nested true
> mode all
> match-field exact:datastore=store
> match-field exact:type=prune
> match-severity error
>
I think I might need to NACK my own patch series here. After some
experimentation and more careful testing, I've become quite unhappy with the
overall UX of these new nested matchers.
For context, my original plan when I first created the notification stack was
to eventually allow to create nested rules by adding new "any/all" sub-nodes in
the match rule tree of a *single* matcher. Each of these new nodes could then
have further 'match-*' rules as their children, etc.
In this series however, I proposed a different approach. Here you could
reference *other* matchers from a matcher's rule tree; the nested matcher
must be created first from the regular matcher overview.
The reason why I went with the current approach were technical
limitations of our tech stack. Neither section-config nor the our POST/PUT
x-www-form-urlencoded data work particularly well for highly nested data
structures, like match rule nodes of a tree.
The current approach was the most straight-forward one to implement,
without requiring major changes to any component in the stack,
neither the UI, nor the API, nor the config format.
I see the following problems with my current approach:
- The matcher overview becomes cluttered quite quickly. For example,
implementing the (legacy) notification policy of a single datastore
(always, never, error) x (GC, Prune, Verify, Sync) results in up to 5
matchers (1 top-level matching 'any' of 4 nested matches, one for each
event type/severity combination). Having multiple datastores with different
policies (e.g. different admins, or production/non-production), makes it
even more cluttered and requires conscious effort by the admin to keep
things easy to understand (e.g. good naming/comments for nested matchers)
- A nested matcher must already exist when being added to a top-level
matcher. Creating matchers 'bottom-up' feels unnatural and might force
users to jump around different matcher config windows to produce the
desired policy.
- Understanding a nested matcher structure requires opening multiple modal
dialogs, hampering usability. There is no way to see the full matcher
hierarchy at once.
I think this should be implemented in a way that aligns more with my
original idea. The core takeaway of these UX problems is that these (nested)
rules should be edited from a single place in the UI, such as the match rule
tree that already exists in the matcher edit window.
I think the most realistic option, keeping the limitations of our API and
section config in mind, is to find a backwards-compatible way to flatten these
nested rules from the tree and store them all in a single 'matcher' section.
Alternatively, we could keep the proposed nested/eval-matcher structure in
the config file and then provide some API to update multiple
matchers in one go (either multiple POST/PUT wrapped in a transaction, or by
having a single endpoint which then 'de-multiplexes' into multiple
matcher sections.
I really wish we could just send a JSON body in the POST/PUT and store the
rules in a config format with proper nesting, this would make all of this
*much* easier, both for API consumers as well as the API/config
implementation.
--
- Lukas
_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel