public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Dominik Csapak <d.csapak@proxmox.com>
To: Lukas Wagner <l.wagner@proxmox.com>,
	Proxmox VE development discussion <pve-devel@lists.proxmox.com>
Subject: Re: [pve-devel] [PATCH v2 many 00/52] revamp notifications; smtp endpoints; system mail
Date: Mon, 20 Nov 2023 10:11:05 +0100	[thread overview]
Message-ID: <987d0f8d-26c1-4c56-ad3f-f2e34c45e678@proxmox.com> (raw)
In-Reply-To: <69565345-fbc5-478c-a2cf-4c83b5f1bba1@proxmox.com>

On 11/20/23 10:03, Lukas Wagner wrote:
> 
> 
> On 11/17/23 09:41, Dominik Csapak wrote:
>> one additional comment to the ux:
>>
>> the match field could use a bit of improvement:
>>
>> the docs describe the current fields, but for users that
>> don't deep dive into the docs first it may be confusing having
>> a simple text input field for that
>>
>> at least when the type is 'exact' we could offer a
>> dropdown for the 'type' value (can still be editable)
> 
> I'm actually evaluating if it would make sense to have a separate
> 'match-type' match rule. That one would use the 'type' field
> in the notification metadata, but would be a bit more obvious to use for the user.
> 
> I think filtering notifications by type is probably going to be one of the most common things a user 
> wants to do. Right now, if a user only
> cares about a subset of notification types, e.g. backups and replication, a matcher would could look 
> like this:
> 
> matcher: test
>      mode any
>      match-field exact:type=vzdump
>      match-field exact:type=replication
> 
> Here, we have to use the 'any' mode, so this is awkward, if we also want to filter based on severity.
> 
> 
> Alternatively, a regex-based solution could be used:
> 
> matcher: test2
>      match-field regex:type=^(vzdump|replication)$
> 
> This requires regex knowledge, and the resulting regular expression can become very long if one 
> wants to match many other types of notifications.
> 
> A 'match-type' rule would simplify this by simply allowing one to list all notification types on 
> wants to match, eg.
> 
> matcher: test3
>      match-type vzdump,replication
> 
> 
> For this 'specialized' match-rule we could easily add a list of known
> values in a drop-down (first hardcoded in the UI, later retrieved via an API call)

maybe having it more generic and making it 'match-list' with a property and multiple
values would make more sense? altough that would be very similar to allow
nesting of modes e.g.

mode all
  - mode any
    - match-field exact:type=foo
    - match-field exact:type=foo
  - match-severity:...

would that be feasible to do (probably not in the short term)?

> 
>>
>> in the mid/long term i think having a backend generated list
>> of those somehow would make sense (i.e. some kind
>> of 'registering' and an endpoint that lists the types,
>> or other metadata) but i think that was planned by you anyway
>> (or something along those lines?)
>>
> 
> Yup, that's on my todo-list, having something like a notification
> registry, where we can register notification types, their templates
> (probably separate templates for plain text/HTML, as I don't quite like
> how rendering from a single template turned out in the end, too finicky and lacking flexibility), 
> and the params needed to render the template.
> That way we can enumerate them in the UI quite easily (and
> also auto-generate the list of notifications in the docs)
> 
>> also it would be good to have a link to the docs on the
>> filter edit panel to the relevant section
>> (e.g. for such things as the types/metadata/etc.)
>>
> Noted, will be added in a follow-up.
> 





  reply	other threads:[~2023-11-20  9:11 UTC|newest]

Thread overview: 65+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-11-14 12:59 Lukas Wagner
2023-11-14 12:59 ` [pve-devel] [PATCH v2 debcargo-conf 01/52] cherry-pick chumsky 0.9.2 from debian unstable Lukas Wagner
2023-11-14 16:15   ` [pve-devel] applied: " Thomas Lamprecht
2023-11-14 12:59 ` [pve-devel] [PATCH v2 debcargo-conf 02/52] update lettre to 0.11.1 Lukas Wagner
2023-11-14 16:15   ` [pve-devel] applied: " Thomas Lamprecht
2023-11-14 12:59 ` [pve-devel] [PATCH v2 proxmox 03/52] notify: introduce Error::Generic Lukas Wagner
2023-11-14 12:59 ` [pve-devel] [PATCH v2 proxmox 04/52] notify: factor out notification content into its own type Lukas Wagner
2023-11-14 12:59 ` [pve-devel] [PATCH v2 proxmox 05/52] notify: replace filters and groups with matcher-based system Lukas Wagner
2023-11-14 12:59 ` [pve-devel] [PATCH v2 proxmox 06/52] notify: add calendar matcher Lukas Wagner
2023-11-14 12:59 ` [pve-devel] [PATCH v2 proxmox 07/52] notify: matcher: introduce common trait for match directives Lukas Wagner
2023-11-14 12:59 ` [pve-devel] [PATCH v2 proxmox 08/52] notify: let a matcher always match if it has no matching directives Lukas Wagner
2023-11-14 12:59 ` [pve-devel] [PATCH v2 proxmox 09/52] sys: email: add `forward` Lukas Wagner
2023-11-14 12:59 ` [pve-devel] [PATCH v2 proxmox 10/52] notify: add mechanisms for email message forwarding Lukas Wagner
2023-11-14 12:59 ` [pve-devel] [PATCH v2 proxmox 11/52] notify: add PVE/PBS context Lukas Wagner
2023-11-14 12:59 ` [pve-devel] [PATCH v2 proxmox 12/52] notify: add 'smtp' endpoint Lukas Wagner
2023-11-14 12:59 ` [pve-devel] [PATCH v2 proxmox 13/52] notify: add api for smtp endpoints Lukas Wagner
2023-11-14 12:59 ` [pve-devel] [PATCH v2 proxmox 14/52] notify: add 'disable' parameter for matchers and targets Lukas Wagner
2023-11-14 12:59 ` [pve-devel] [PATCH v2 proxmox 15/52] notify: add built-in config and 'origin' parameter Lukas Wagner
2023-11-14 12:59 ` [pve-devel] [PATCH v2 proxmox-perl-rs 16/52] notify: adapt to new matcher-based notification routing Lukas Wagner
2023-11-14 12:59 ` [pve-devel] [PATCH v2 proxmox-perl-rs 17/52] notify: add bindings for smtp API calls Lukas Wagner
2023-11-14 12:59 ` [pve-devel] [PATCH v2 proxmox-perl-rs 18/52] pve-rs: notify: remove notify_context for PVE Lukas Wagner
2023-11-14 12:59 ` [pve-devel] [PATCH v2 proxmox-perl-rs 19/52] notify: add 'disable' parameter Lukas Wagner
2023-11-14 12:59 ` [pve-devel] [PATCH v2 proxmox-perl-rs 20/52] notify: support 'origin' paramter Lukas Wagner
2023-11-14 12:59 ` [pve-devel] [PATCH v2 pve-cluster 21/52] notify: adapt to matcher based notification system Lukas Wagner
2023-11-14 12:59 ` [pve-devel] [PATCH v2 pve-guest-common 22/52] vzdump: deprecate mailto/mailnotification/notification-{target, policy} Lukas Wagner
2023-11-14 12:59 ` [pve-devel] [PATCH v2 pve-ha-manager 23/52] env: switch to matcher-based notification system Lukas Wagner
2023-11-14 12:59 ` [pve-devel] [PATCH v2 pve-manager 24/52] api: notification: remove notification groups Lukas Wagner
2023-11-14 12:59 ` [pve-devel] [PATCH v2 pve-manager 25/52] api: notification: add new matcher-based notification API Lukas Wagner
2023-11-14 12:59 ` [pve-devel] [PATCH v2 pve-manager 26/52] ui: dc: remove unneeded notification events panel Lukas Wagner
2023-11-14 12:59 ` [pve-devel] [PATCH v2 pve-manager 27/52] vzdump: adapt to new matcher based notification system Lukas Wagner
2023-11-14 12:59 ` [pve-devel] [PATCH v2 pve-manager 28/52] api: apt: adapt to matcher-based notifications Lukas Wagner
2023-11-14 12:59 ` [pve-devel] [PATCH v2 pve-manager 29/52] api: replication: adapt to matcher-based notification system Lukas Wagner
2023-11-14 12:59 ` [pve-devel] [PATCH v2 pve-manager 30/52] test: fix vzdump notification test Lukas Wagner
2023-11-14 12:59 ` [pve-devel] [PATCH v2 pve-manager 31/52] ui: vzdump: remove left-overs from target/policy based notifications Lukas Wagner
2023-11-16 11:52   ` [pve-devel] [PATCH manager] ui: fix backup job create Dominik Csapak
2023-11-14 12:59 ` [pve-devel] [PATCH v2 pve-manager 32/52] ui: dc: config: show notification panel again Lukas Wagner
2023-11-14 12:59 ` [pve-devel] [PATCH v2 pve-manager 33/52] notify: add API routes for smtp endpoints Lukas Wagner
2023-11-14 12:59 ` [pve-devel] [PATCH v2 pve-manager 34/52] api: notification: add disable and origin params Lukas Wagner
2023-11-14 12:59 ` [pve-devel] [PATCH v2 pve-manager 35/52] api: notification: simplify ACLs for notification Lukas Wagner
2023-11-14 12:59 ` [pve-devel] [PATCH v2 proxmox-widget-toolkit 36/52] notification ui: add target selector for matcher Lukas Wagner
2023-11-14 12:59 ` [pve-devel] [PATCH v2 proxmox-widget-toolkit 37/52] notification ui: remove filter setting for targets Lukas Wagner
2023-11-14 12:59 ` [pve-devel] [PATCH v2 proxmox-widget-toolkit 38/52] notification ui: remove notification groups Lukas Wagner
2023-11-14 12:59 ` [pve-devel] [PATCH v2 proxmox-widget-toolkit 39/52] notification ui: rename filter to matcher Lukas Wagner
2023-11-14 12:59 ` [pve-devel] [PATCH v2 proxmox-widget-toolkit 40/52] notification: matcher: add UI for matcher editing Lukas Wagner
2023-11-14 12:59 ` [pve-devel] [PATCH v2 proxmox-widget-toolkit 41/52] notification ui: unprotected mailto-root target Lukas Wagner
2023-11-14 12:59 ` [pve-devel] [PATCH v2 proxmox-widget-toolkit 42/52] noficiation: matcher edit: make 'field' an editable combobox Lukas Wagner
2023-11-14 12:59 ` [pve-devel] [PATCH v2 proxmox-widget-toolkit 43/52] panel: notification: add gui for SMTP endpoints Lukas Wagner
2023-11-14 12:59 ` [pve-devel] [PATCH v2 proxmox-widget-toolkit 44/52] notification ui: add enable checkbox for targets/matchers Lukas Wagner
2023-11-14 12:59 ` [pve-devel] [PATCH v2 proxmox-widget-toolkit 45/52] notification ui: add column for 'origin' Lukas Wagner
2023-11-14 12:59 ` [pve-devel] [PATCH v2 pve-docs 46/52] notifications: update docs to for matcher-based notifications Lukas Wagner
2023-11-14 12:59 ` [pve-devel] [PATCH v2 pve-docs 47/52] notifications: document SMTP endpoints Lukas Wagner
2023-11-14 12:59 ` [pve-devel] [PATCH v2 pve-docs 48/52] notifications: document 'comment' option for targets/matchers Lukas Wagner
2023-11-14 12:59 ` [pve-devel] [PATCH v2 pve-docs 49/52] notifications: add documentation for system mail forwarding Lukas Wagner
2023-11-14 12:59 ` [pve-devel] [PATCH v2 pve-docs 50/52] notifications: change to simplified ACL structure Lukas Wagner
2023-11-14 12:59 ` [pve-devel] [PATCH v2 proxmox-mail-forward 51/52] feed forwarded mails into proxmox_notify Lukas Wagner
2023-11-14 13:00 ` [pve-devel] [PATCH v2 proxmox-mail-forward 52/52] update d/control Lukas Wagner
2023-11-16 11:57 ` [pve-devel] [PATCH widget-toolkit 0/2] follow-ups for notification series Dominik Csapak
2023-11-16 11:57   ` [pve-devel] [PATCH widget-toolkit 1/2] notification matcher: improve handling empty and invalid values Dominik Csapak
2023-11-16 11:57   ` [pve-devel] [PATCH widget-toolkit 2/2] notification matcher: improve wording for mode Dominik Csapak
2023-11-17  8:41 ` [pve-devel] [PATCH v2 many 00/52] revamp notifications; smtp endpoints; system mail Dominik Csapak
2023-11-20  9:03   ` Lukas Wagner
2023-11-20  9:11     ` Dominik Csapak [this message]
2023-11-20  9:49       ` Lukas Wagner
2023-11-20  9:11     ` Thomas Lamprecht
2023-11-17 15:31 ` [pve-devel] applied-series: " Thomas Lamprecht

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=987d0f8d-26c1-4c56-ad3f-f2e34c45e678@proxmox.com \
    --to=d.csapak@proxmox.com \
    --cc=l.wagner@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