Re: [PATCH proxmox v4 15/39] notify: migrate PBS's and PVE's default matcher to expression syntax

["Michael Köppl" <m.koeppl@proxmox.com>]

On Thu Oct 22, 2026 at 9:35 AM CEST, Lukas Wagner wrote:
> On Wed Oct 21, 2026 at 4:39 PM CEST, Michael Köppl wrote:
>> On Mon Oct 12, 2026 at 3:13 PM CEST, Lukas Wagner wrote:
>>
>> [snip]
>>
>>> +            use proxmox_match_expression::{not, any_of, all_of};
>>> +
>>> +            let expression = any_of![
>>> +                not!(
>>> +                    FieldMatcher::Exact {
>>> +                        field: "type".into(),
>>> +                        values: vec!["prune".into()],
>>> +                    }.into()
>>> +                ),
>>> +                all_of![
>>> +                    FieldMatcher::Exact {
>>> +                        field: "type".into(),
>>> +                        values: vec!["prune".into()],
>>> +                    }.into(),
>>> +                    SeverityMatcher {
>>> +                        severities: vec![Severity::Error, Severity::Warning, Severity::Notice],
>>
>> This is more of a question than actual feedback, I think, but how likely
>> is it that additional severity levels could be added in the future?
>
> Not impossible (e.g. maybe we'd want to have 'critical' at one point, to
> differentiate 'hey, your cluster is on fire' from a mere failed backup),
> but also not something that is on the immediate horizon, as far as I'm
> concerned.
>
>> Because the old default matcher was
>> not(type == prune AND severity == info), whereas this specifically
>> matches a list of severities that would need to be extended to remain
>> equivalent, no?
>>
>> In any case, couldn't this simply be
>>
>> ```
>> let expression = not!(all_of![
>> 	FieldMatcher::Exact {
>> 		field: "type".into(),
>> 		values: vec!["prune".into()],
>> 	}.into(),
>> 	SeverityMatcher {
>> 		severities: vec![Severity::Info],
>> 	}.into(),
>> ]);
>> ```
>>
>> to be equivalent to the old one? Or is there a reason this is not the
>> case?
>
> The main reason why I changed the representation was that the old one
> was really quite difficult to understand; I've asked multiple people in
> a poll and they all preferred representation used in this patch.
>

Yeah, true, I forgot about that. As I recall, I was even one of the
people who found the new representation more intuitive (and looking at
it again now, I still think so).

> I don't view the default matcher to be set in stone anyway, so having to
> change it if we add more severity levels does not really concern me.
>
> A patch series I'm currently working on moves the definition to the
> application code, which makes it also easier and quicker to adapt in the
> future.

Sounds good! Sorry for the noise then :)