Re: [pbs-devel] [PATCH proxmox-backup 00/33] integrate notification system

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

Hey! Thanks a lot for this first round of feedback!

On 12.04.24 15:59, Gabriel Goller wrote:
> First of all, thanks for implementing this! I can't count how many times
> I've seen a forum post asking for better notifications in pbs!
> 
> Some high-level comments:
>  - The 'hostname' field doesn't make any sense in pbs (AFAICT) because
>    there is only one node.

Right, we might just kick that out then.

>  - The 'type' field could have hardcoded values, such as 'gc', 'verify',
>    etc. this way we can also have a drop-down menu in the ui. This makes
>    it a lot easier than checking the docs/guessing.

If you apply/deploy the `proxmox-widget-toolkit` patches from
https://lists.proxmox.com/pipermail/pve-devel/2024-February/061992.html

then you should automatically get an improved UI for creating matchers
where one can select from known types and other metadata values. :)


>  - A notification when a backup succeeds/fails would be very useful as
>    well.

For now I have just ported the existing notifications - new ones
can of course be added later :)

>  - Also having datastore and namespace filters on the notification
>    matchers would be great (if feasible obv.).

Matching on datastores should also work already with the WTK-patches
mentioned above

> 
> 
> Another thing I noticed is that creating a datastore through the cli 
> still uses the legacy notification system.
> 

Ah, thx!

> Will make a more in depth review next week!
> 

Greatly appreciated!