public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Arthur Bied-Charreton <a.bied-charreton@proxmox.com>
To: Lukas Wagner <l.wagner@proxmox.com>
Cc: pbs-devel@lists.proxmox.com, pve-devel@lists.proxmox.com
Subject: Re: [PATCH many 00/29] notifications: add nested match expressions
Date: Fri, 24 Jul 2026 08:44:54 +0200	[thread overview]
Message-ID: <7ozvhk2v3zjg62jhrosyurbus2yjajtglsmlj4gfswr43ekoub@rghnsykkbv3p> (raw)
In-Reply-To: <20260709115716.299836-1-l.wagner@proxmox.com>

On Thu, Jul 09, 2026 at 01:56:47PM +0200, Lukas Wagner wrote:
> NOTE: I have a couple of other patches planned for the notification stack (**);
> so I guess it would make sense wait a bit before applying this -- just to avoid
> too many version bumps and too much maintainer churn.
> 
> This resolves a long-standing limitation of notification matchers. Up until
> now, notification match rules could only be one level deep, so one top-level
> combinator (all/any) and an arbitrary number of match rules as direct children.
> This severely limits which kind of matching behaviors can be represented.
> 
> The notification stack was always designed with the possibility of arbitrarily
> nested match rules, however, due to limitations in our API and config formats
> (lacking support for nested data structures), this was not implemented yet.
> 
> A prior attempt to resolve the limitations imposed by the lack of nesting was
> to add 'sub-matchers', where one matcher could evaluate the result of another
> match. After some consideration, I decided not to pursue this approach further,
> due to concerns about the UX of that approach [1].
> 
> This new series chooses a different approach. Nested match rules can now be
> represented by a single matcher in the config. The lack of support for nested
> data structures in the API was circumvented by storing a serialized form of the
> 'rule tree' as a JSON blob in a single new configuration parameter
> 'expression'. If this new key is set, the old 'match-*', 'invert-match' and
> 'mode' keys are ignored; 'expression' is intended as a replacement for these
> keys. I have not yet marked the old keys as deprecated. Theoretically these can
> stay supported for now, however when editing a matcher via the UI, the old keys
> will always be translated into an equivalent 'expression'. For any new products
> receiving the notification stack, we can drop the support for the old keys by
> compiling proxmox-notify without the 'legacy-matchers' flag.
> 
> The expression evaluation logic was implemented in a new generic crate
> 'proxmox-match-expression'. 'proxmox-notify' is the first user of this crate,
> however it is generic enough to be potentially useful in other contexts as
> well.
> 
> Bumps:
>   - pve-rs needs proxmox-notify bumped
>   - pve-manager needs pve-rs and widget-toolkit bumped
>   - proxmox-backup needs widget-toolkit and proxmox-notify bumped
>   - proxmox-notify needs a first release of proxmox-match-expression
>   
>   - proxmox-mail-forward needs to be rebuilt against bumped proxmox-notify (unless
>     it has already been changed to not call into proxmox-notify anymore, see below)
> 
> [1] https://lore.proxmox.com/pbs-devel/20250521142309.264719-1-l.wagner@proxmox.com/
> 
> 
> (**): Some of the things I want to improve/implement:
>    - Migrate PVE to use a worker-based approach, same as in PBS
>      -> once that is done, we can fully migrate proxmox-mail-forward
>         to use the worker/spool dir approach, instead of using proxmox-notify
>         directly
>    - Introduce a notification history
>    - Refactor error types in proxmox-notify a bit
> 
> Also, Arthur's XOAUTH2 patch series should probably be applied in one go with
> my efforts. He probably needs to rebase on top of my changes anyway.
> 
[...]

thanks a lot for working on this! this is a  very nice improvement to 
the notification stack UX. 

i tested backwards compatibility with older configs and the migration to
the new expression format on R/W cycles, it works really well and the
generated new format is semantically equivalent fwict. old configs that
have not yet been migrated still work as expected. 

broken matcher expressions only abort evaluation for that matcher, not
for the whole notification. 

old rules are ignored if 'expression' is set, as advertised. 

i replied to some patches with some comments/questions, overall i think
this is already in great shape, nice work!!

[0] https://pve.proxmox.com/pve-docs/chapter-notifications.html#notification_matchers_calendar
[1] https://lore.proxmox.com/pve-devel/20260723114258.316106-1-a.bied-charreton@proxmox.com/T/#u
[2] https://lore.proxmox.com/pbs-devel/20260723114548.319543-1-a.bied-charreton@proxmox.com/T/#u




  parent reply	other threads:[~2026-07-24  6:45 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-09 11:56 [PATCH many 00/29] notifications: add nested match expressions Lukas Wagner
2026-07-09 11:56 ` [PATCH proxmox 01/29] add new proxmox-match-expression crate Lukas Wagner
2026-07-24  6:46   ` Arthur Bied-Charreton
2026-07-09 11:56 ` [PATCH proxmox 02/29] notify: promote matcher to dir-style module Lukas Wagner
2026-07-09 11:56 ` [PATCH proxmox 03/29] notify: fix doc comment Lukas Wagner
2026-07-09 11:56 ` [PATCH proxmox 04/29] notify: matcher: break out severity matcher into submodule Lukas Wagner
2026-07-09 11:56 ` [PATCH proxmox 05/29] notify: matcher: break out field " Lukas Wagner
2026-07-09 11:56 ` [PATCH proxmox 06/29] notify: matcher: break out calendar " Lukas Wagner
2026-07-09 11:56 ` [PATCH proxmox 07/29] notify: matcher: calendar: add basic unit test Lukas Wagner
2026-07-09 11:56 ` [PATCH proxmox 08/29] notify: matcher: add InlineSeverityMatcher Lukas Wagner
2026-07-09 11:56 ` [PATCH proxmox 09/29] notify: matcher: add InlineFieldMatcher Lukas Wagner
2026-07-09 11:56 ` [PATCH proxmox 10/29] notify: matcher: add InlineCalendarMatcher Lukas Wagner
2026-07-24  6:47   ` Arthur Bied-Charreton
2026-07-24  9:43     ` Wolfgang Bumiller
2026-07-09 11:56 ` [PATCH proxmox 11/29] notify: matcher: add expression support Lukas Wagner
2026-07-24  6:59   ` Arthur Bied-Charreton
2026-07-24  8:27     ` PVE::Cluster::cfs_lock_domain - bizarre issue Anthony Galica
2026-07-24  9:38       ` :Cluster::cfs_lock_domain " Anthony Galica
2026-07-25  7:14         ` Anthony Galica
2026-07-09 11:56 ` [PATCH proxmox 12/29] notify: api: support new expression parameter Lukas Wagner
2026-07-24  6:46   ` Arthur Bied-Charreton
2026-07-24 10:01     ` Wolfgang Bumiller
2026-07-24 11:38       ` Arthur Bied-Charreton
2026-07-09 11:57 ` [PATCH proxmox 13/29] notify: api: add `get_matcher_as_expression` Lukas Wagner
2026-07-09 11:57 ` [PATCH proxmox 14/29] notify: migrate PBS's and PVE's default matcher to expression syntax Lukas Wagner
2026-07-24  6:47   ` Arthur Bied-Charreton
2026-07-09 11:57 ` [PATCH proxmox 15/29] notify: move legacy matcher keys behind feature flag Lukas Wagner
2026-07-09 11:57 ` [PATCH proxmox-widget-toolkit 16/29] notification: increase matcher window width Lukas Wagner
2026-07-09 11:57 ` [PATCH proxmox-widget-toolkit 17/29] notifications: matcher: add support for match expressions Lukas Wagner
2026-07-24  6:45   ` Arthur Bied-Charreton
2026-07-09 11:57 ` [PATCH proxmox-widget-toolkit 18/29] notification: matcher: add better calendar editor Lukas Wagner
2026-07-24  6:46   ` Arthur Bied-Charreton
2026-07-09 11:57 ` [PATCH proxmox-widget-toolkit 19/29] notifications: matcher: consistently use title case for UI elements Lukas Wagner
2026-07-09 11:57 ` [PATCH proxmox-backup 20/29] notification: opt into 'legacy-matchers' feature in proxmox-notify Lukas Wagner
2026-07-09 11:57 ` [PATCH proxmox-backup 21/29] api: notification: add 'migrate-to-expression' parameter to get_matcher Lukas Wagner
2026-07-09 11:57 ` [PATCH proxmox-backup 22/29] ui: notification: enable new matcher UI Lukas Wagner
2026-07-09 11:57 ` [PATCH proxmox-perl-rs 23/29] notify: matcher: pass matcher config / updater directly Lukas Wagner
2026-07-09 11:57 ` [PATCH proxmox-perl-rs 24/29] notify: opt into 'legacy-matchers' feature in proxmox-notify Lukas Wagner
2026-07-24  6:48   ` Arthur Bied-Charreton
2026-07-09 11:57 ` [PATCH proxmox-perl-rs 25/29] notify: add 'migrate_to_expression' parameter for get_matcher Lukas Wagner
2026-07-09 11:57 ` [PATCH manager 26/29] api: notification: pass config/updater directly to rust bindings Lukas Wagner
2026-07-09 11:57 ` [PATCH manager 27/29] api: notification: get_matcher: add 'migrate-to-expression' parameter Lukas Wagner
2026-07-09 11:57 ` [PATCH manager 28/29] api: notification: add 'expression' to matcher parameter schema Lukas Wagner
2026-07-09 11:57 ` [PATCH manager 29/29] ui: notification: enable new matcher UI Lukas Wagner
2026-07-24  6:44 ` Arthur Bied-Charreton [this message]
2026-07-24  6:53   ` [PATCH many 00/29] notifications: add nested match expressions Arthur Bied-Charreton

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=7ozvhk2v3zjg62jhrosyurbus2yjajtglsmlj4gfswr43ekoub@rghnsykkbv3p \
    --to=a.bied-charreton@proxmox.com \
    --cc=l.wagner@proxmox.com \
    --cc=pbs-devel@lists.proxmox.com \
    --cc=pve-devel@lists.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal