Re: [PATCH many v2 00/34] notifications: add nested match expressions

["Lukas Wagner" <l.wagner@proxmox.com>]

On Wed Oct 7, 2026 at 11:32 AM CEST, Michael Köppl wrote:
> Tested this on a 3-node cluster:
> - upgrading the nodes one after another, checking in between what
>   happens when an upgraded node persists config changes (see comment on
>   15/34)
> - playing with various different expressions through both the UI and
>   pvesh, combining sub-rules and checking that the "quantifiers" for
>   the sub-rules work as expected
>
> Wanted to note a few things I noticed so far, both not a big deal:
> - Is it on purpose that after migrating to the new config format, the
>   origin of the default-matcher is shown as "built-in modified"? Isn't
>   it a bit misleading to show it as modified if the user never changed
>   anything?

It shows up as modified if the entity on disk is different from the one
that is baked into the application.

In this series, we changed the default, so I think it makes sense if the
config is shown as modified in these circumstances. What makes it
slightly confusing is that the matching *behavior* stayed the same.

Maybe I could change the code in proxmox-notify in such a way that
not-modified built-ins are never serialized to the config, then any new
default would be automatically picked up, and it would not be shown as
modified.

But I don't think this should be a blocker for this series, the
'modified' state is purely cosmetic here, it does not influence any
behavior of the system.


> - Entering 0:00 instead of 00:00 for the Match Calender rule type
>   highlights the input field in red, but the "Add" button can still be
>   pressed and a request will be sent, attempting to add a Match
>   Calendar rule with -23:59, resulting in an error message from the
>   backend.
>

Will be fixed in v3, thanks!

> Some other observations I added on the individual patches since I've
> been looking into  the implementation anyway.
>
> On Thu Oct 1, 2026 at 10:36 AM CEST, 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. I have a patch series
>> already in the works that builds on top of this one, so I would prefer to
>> perform any non-trivial changes to this feature as follow-ups; rebasing the new
>> series I'm working on right now is a huge pain, as it moves a lot of code
>> around in proxmox-notify (preparations for proxmox-notify v2, essentially)
>>
>
> [snip]