From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id D00C56BEF6 for ; Fri, 19 Mar 2021 07:07:58 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id C2D791F81E for ; Fri, 19 Mar 2021 07:07:58 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [212.186.127.180]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id 753CA1F80E for ; Fri, 19 Mar 2021 07:07:57 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 45DA64288B for ; Fri, 19 Mar 2021 07:07:57 +0100 (CET) Date: Fri, 19 Mar 2021 07:07:29 +0100 (CET) From: Dietmar Maurer To: Proxmox Backup Server development discussion , Dominik Csapak Message-ID: <1789734810.421.1616134050079@webmail.proxmox.com> In-Reply-To: <20210318120108.26423-5-d.csapak@proxmox.com> References: <20210318120108.26423-1-d.csapak@proxmox.com> <20210318120108.26423-5-d.csapak@proxmox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Priority: 3 Importance: Normal X-Mailer: Open-Xchange Mailer v7.10.4-Rev20 X-Originating-Client: open-xchange-appsuite X-SPAM-LEVEL: Spam detection results: 0 AWL 0.110 Adjusted score from AWL reputation of From: address KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [mod.rs, backup.rs] Subject: Re: [pbs-devel] [PATCH proxmox-backup v2 4/5] api2/tape/backup: include a summary on notification e-mails X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 19 Mar 2021 06:07:58 -0000 comments inline > On 03/18/2021 1:01 PM Dominik Csapak wrote: > > > for now only contains the list of included snapshots (if any), > as well as the backup duration > > Signed-off-by: Dominik Csapak > --- > src/api2/tape/backup.rs | 32 +++++++++++++++++++++++++------ > src/api2/types/tape/mod.rs | 9 +++++++++ > src/server/email_notifications.rs | 13 +++++++++++++ > 3 files changed, 48 insertions(+), 6 deletions(-) > > diff --git a/src/api2/tape/backup.rs b/src/api2/tape/backup.rs > index 690b6855..0d42c31a 100644 > --- a/src/api2/tape/backup.rs > +++ b/src/api2/tape/backup.rs > @@ -51,6 +51,7 @@ use crate::{ > JOB_ID_SCHEMA, > MediaPoolConfig, > Userid, > + TapeBackupJobSummary, > }, > server::WorkerTask, > task::TaskState, > @@ -198,13 +199,16 @@ pub fn do_tape_backup_job( > let notify_user = setup.notify_user.as_ref().unwrap_or_else(|| &Userid::root_userid()); > let email = lookup_user_email(notify_user); > > - let job_result = backup_worker( > + let (job_result, summary) = match backup_worker( > &worker, > datastore, > &pool_config, > &setup, > email.clone(), > - ); > + ) { > + Ok(summary) => (Ok(()), summary), > + Err(err) => (Err(err), Default::default()), > + }; > > let status = worker.create_state(&job_result); > > @@ -214,6 +218,7 @@ pub fn do_tape_backup_job( > Some(job.jobname()), > &setup, > &job_result, > + summary, > ) { > eprintln!("send tape backup notification failed: {}", err); > } > @@ -340,13 +345,17 @@ pub fn backup( > move |worker| { > let _drive_lock = drive_lock; // keep lock guard > set_tape_device_state(&setup.drive, &worker.upid().to_string())?; > - let job_result = backup_worker( > + > + let (job_result, summary) = match backup_worker( > &worker, > datastore, > &pool_config, > &setup, > email.clone(), > - ); > + ) { > + Ok(summary) => (Ok(()), summary), > + Err(err) => (Err(err), Default::default()), > + }; > > if let Some(email) = email { > if let Err(err) = crate::server::send_tape_backup_status( > @@ -354,6 +363,7 @@ pub fn backup( > None, > &setup, > &job_result, > + summary, > ) { > eprintln!("send tape backup notification failed: {}", err); > } > @@ -374,9 +384,11 @@ fn backup_worker( > pool_config: &MediaPoolConfig, > setup: &TapeBackupJobSetup, > email: Option, > -) -> Result<(), Error> { > +) -> Result { > > let status_path = Path::new(TAPE_STATUS_DIR); > + let start = std::time::Instant::now(); > + let mut summary: TapeBackupJobSummary = Default::default(); > > let _lock = MediaPool::lock(status_path, &pool_config.name)?; > > @@ -422,8 +434,11 @@ fn backup_worker( > task_log!(worker, "skip snapshot {}", info.backup_dir); > continue; > } > + let snapshot_name = info.backup_dir.to_string(); > if !backup_snapshot(worker, &mut pool_writer, datastore.clone(), info.backup_dir)? { > errors = true; > + } else { > + summary.snapshot_list.push(snapshot_name); > } > progress.done_snapshots = 1; > task_log!( > @@ -439,8 +454,11 @@ fn backup_worker( > task_log!(worker, "skip snapshot {}", info.backup_dir); > continue; > } > + let snapshot_name = info.backup_dir.to_string(); > if !backup_snapshot(worker, &mut pool_writer, datastore.clone(), info.backup_dir)? { > errors = true; > + } else { > + summary.snapshot_list.push(snapshot_name); > } > progress.done_snapshots = snapshot_number as u64 + 1; > task_log!( > @@ -464,7 +482,9 @@ fn backup_worker( > bail!("Tape backup finished with some errors. Please check the task log."); > } > > - Ok(()) > + summary.duration = start.elapsed(); > + > + Ok(summary) > } > > // Try to update the the media online status > diff --git a/src/api2/types/tape/mod.rs b/src/api2/types/tape/mod.rs > index 68b2cf12..44c2236b 100644 > --- a/src/api2/types/tape/mod.rs > +++ b/src/api2/types/tape/mod.rs > @@ -20,3 +20,12 @@ pub use media_location::*; > > mod media; > pub use media::*; > + > +/// Summary of a successful Tape Job > +#[derive(Default)] > +pub struct TapeBackupJobSummary { > + /// The list of snaphots backed up > + pub snapshot_list: Vec, > + /// The total time of the backup job > + pub duration: std::time::Duration, > +} This is not an API type, and is not use in the API at all! Please can you define this type somewhere else?