Re: [PATCH proxmox-perl-rs 24/29] notify: opt into 'legacy-matchers' feature in proxmox-notify

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

On Wed Sep 23, 2026 at 2:32 PM CEST, Lukas Wagner wrote:
>>> +proxmox-notify = { version = "1", features = ["pve-context", "legacy-matchers"] }
>> afaict this should also be added to pmg-rs. it currently does not
>> compile otherwise. 
>
> Good catch!
>
> This is a bit weird, since PMG currently does not use proxmox-notify at
> all. If we ever add the notification stack to PMG (not planned for the
> near future, but maybe some day), we'd likely not want to pull in the
> legacy-matchers feature at all.
>
> For now, I've solved this by adding a 'notify-legacy-matchers' feature
> in pmg-rs and pve-rs, which is only enabled by default in pve-rs.
>
> Unfortunately, in perlmod modules it does not seem to be possible to
> use feature gates on functions, so I had to adapt get_matcher as follows:
>
>     #[export(serialize_error)]
>     pub fn get_matcher(
>         #[try_from_ref] this: &NotificationConfig,
>         id: &str,
>         #[allow(unused)] migrate_to_expression: Option<bool>,
>     ) -> Result<MatcherConfig, HttpError> {
>         let config = this.config.lock().unwrap();
>
>         #[cfg(feature = "notify-legacy-matchers")]
>         if migrate_to_expression.unwrap_or_default() {
>             return api::matcher::get_matcher_as_expression(&config, id);
>         }
>
>         api::matcher::get_matcher(&config, id)
>     }
>
> Maybe not the cleanest option, since pmg could then pass a parameter
> that is completely ignored, but i'm not sure if I can think of a better
> solution at the moment (besides moving the notify module into pve-rs,
> but that'd require changing all users to use PVE::RS::Notify instead of
> Proxmox::RS::Notify).


Discussed this issue with Wolfgang, we came to the conclusion that it's
actually better to move the binding code to PVE::RS::Notify, there are
only two call sites that need to be adapted anyway.