Re: [pbs-devel] [PATCH proxmox-backup] implement prune notifications

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

Conflicts, but is auto-, but there's a compile error (see inline).

Am 12/07/2022 um 12:22 schrieb Dominik Csapak:
> we converted the prune settings of datastores to prune-jobs, but did
> not actually implement the notifications for them, even though
> we had the notification options in the gui (they did not work).
> 
> implement the basic ok/error notification for prune jobs
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
> if we want we could collect some basic info about the pruning,
> like how many snapshots we removed/kept

can always add that in the future

> 
> adding the whole list of which snapshots were kept/deleted does
> not really make sense to me though, thats the purpose of the task log
>>  pbs-api-types/src/jobs.rs         |  6 +++
>  src/api2/config/datastore.rs      |  1 +
>  src/server/email_notifications.rs | 74 +++++++++++++++++++++++++++++++
>  src/server/prune_job.rs           |  9 ++++
>  www/window/NotifyOptions.js       |  2 +-
>  5 files changed, 91 insertions(+), 1 deletion(-)
> 

> +pub fn send_prune_status(
> +    email: &str,

fwiw, we could use Option<&str> here and re-use/extend the existing match+early return
to avoid caller headache. We even could drop it completely and call the
lookup_datastore_notify_settings internally, if we ever got other call sites
we can still re-factor it out again or have two methods, an auto variant and a
manual one.

> diff --git a/src/server/prune_job.rs b/src/server/prune_job.rs
> index a62177e4..e9631183 100644
> --- a/src/server/prune_job.rs
> +++ b/src/server/prune_job.rs
> @@ -169,6 +169,8 @@ pub fn do_prune_job(
>          None => format!("{store}"),
>      };
>  
> +    let (email, notify) = crate::server::lookup_datastore_notify_settings(&store);
> +
>      let upid_str = WorkerTask::new_thread(
>          &worker_type,
>          Some(worker_id),
> @@ -191,6 +193,13 @@ pub fn do_prune_job(
>                  eprintln!("could not finish job state for {}: {}", job.jobtype(), err);
>              }
>  
> +            if let Some(email) = email {
> +                if let Err(err) =
> +                    crate::server::send_prune_status(&email, notify, &store, job.jobname(), &result)


compilation errors because your referencing store, which got moved into worker_id above,
simplest thing may be to clone there, I mean we could use &worker.upid().worker_id (it's
pub) but  that would be rather a bit ugly IMO.

> +                {
> +                    eprintln!("send prune notification failed: {}", err);

You got that probably from context, but we normally use the log crates fn's over
eprintln for new code IIRC.

> +                }
> +            }
>              result
>          },
>      )?;