[pbs-devel] [PATCH proxmox-backup] notifications: fix legacy sync notifications

[pbs-devel] [PATCH proxmox-backup] notifications: fix legacy sync notifications

[Gabriel Goller]

When using the legacy notifications the sync mode would pick up the
settings from the prune-job, which default to Error. This completely
disables notifications for successful sync-jobs when using the legacy
system.

Motivation: https://forum.proxmox.com/threads/legacy-notification-does-not-send-some-emails.147018/

Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
 src/server/notifications.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/server/notifications.rs b/src/server/notifications.rs
index d142971b..eea55202 100644
--- a/src/server/notifications.rs
+++ b/src/server/notifications.rs
@@ -332,7 +332,7 @@ pub fn send_sync_status(job: &SyncJobConfig, result: &Result<(), Error>) -> Resu
     let (email, notify, mode) = lookup_datastore_notify_settings(&job.store);
     match mode {
         NotificationMode::LegacySendmail => {
-            let notify = notify.prune.unwrap_or(Notify::Error);
+            let notify = notify.sync.unwrap_or(Notify::Always);
 
             if notify == Notify::Never || (result.is_ok() && notify == Notify::Error) {
                 return Ok(());
-- 
2.43.0



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH proxmox-backup] notifications: fix legacy sync notifications

[Max Carrara]

On Wed May 15, 2024 at 11:58 AM CEST, Gabriel Goller wrote:
> When using the legacy notifications the sync mode would pick up the
> settings from the prune-job, which default to Error. This completely
> disables notifications for successful sync-jobs when using the legacy
> system.
>
> Motivation: https://forum.proxmox.com/threads/legacy-notification-does-not-send-some-emails.147018/
>
> Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
> ---

Just gave this a spin:

* Set up a datastore on my PBS test instance
  (Options: legacy Emailnotifs, notify Sync=Always) 
* Ran a sync job against another local datastore
* Notification is actually sent now, as expected

To be sure, I also tested the "inverse" (as in, without applying this
patch) on my actual PBS instance with another local sync job. Didn't
receive a notification.

So, it's obvious that this patch really fixes those notifs. Good job!

Tested-by: Max Carrara <m.carrara@proxmox.com>

>  src/server/notifications.rs | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/src/server/notifications.rs b/src/server/notifications.rs
> index d142971b..eea55202 100644
> --- a/src/server/notifications.rs
> +++ b/src/server/notifications.rs
> @@ -332,7 +332,7 @@ pub fn send_sync_status(job: &SyncJobConfig, result: &Result<(), Error>) -> Resu
>      let (email, notify, mode) = lookup_datastore_notify_settings(&job.store);
>      match mode {
>          NotificationMode::LegacySendmail => {
> -            let notify = notify.prune.unwrap_or(Notify::Error);
> +            let notify = notify.sync.unwrap_or(Notify::Always);
>  
>              if notify == Notify::Never || (result.is_ok() && notify == Notify::Error) {
>                  return Ok(());



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH proxmox-backup] notifications: fix legacy sync notifications

[Lukas Wagner]

Did not test this, but code-wise this looks good. 
Likely a copy-paste mistake which slipped through code review.

Reviewed-by: Lukas Wagner <l.wagner@proxmox.com>

On  2024-05-15 11:58, Gabriel Goller wrote:
> When using the legacy notifications the sync mode would pick up the
> settings from the prune-job, which default to Error. This completely
> disables notifications for successful sync-jobs when using the legacy
> system.
> 
> Motivation: https://forum.proxmox.com/threads/legacy-notification-does-not-send-some-emails.147018/
> 
> Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
> ---
>  src/server/notifications.rs | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/server/notifications.rs b/src/server/notifications.rs
> index d142971b..eea55202 100644
> --- a/src/server/notifications.rs
> +++ b/src/server/notifications.rs
> @@ -332,7 +332,7 @@ pub fn send_sync_status(job: &SyncJobConfig, result: &Result<(), Error>) -> Resu
>      let (email, notify, mode) = lookup_datastore_notify_settings(&job.store);
>      match mode {
>          NotificationMode::LegacySendmail => {
> -            let notify = notify.prune.unwrap_or(Notify::Error);
> +            let notify = notify.sync.unwrap_or(Notify::Always);
>  
>              if notify == Notify::Never || (result.is_ok() && notify == Notify::Error) {
>                  return Ok(());

-- 
- Lukas


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] applied: [PATCH proxmox-backup] notifications: fix legacy sync notifications

[Thomas Lamprecht]

Am 15/05/2024 um 11:58 schrieb Gabriel Goller:
> When using the legacy notifications the sync mode would pick up the
> settings from the prune-job, which default to Error. This completely
> disables notifications for successful sync-jobs when using the legacy
> system.
> 
> Motivation: https://forum.proxmox.com/threads/legacy-notification-does-not-send-some-emails.147018/
> 
> Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
> ---
>  src/server/notifications.rs | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
>

applied, with Max's T-b and Lukas' R-b, thanks!


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel