Re: [pbs-devel] [PATCH proxmox-backup 1/2] tape/send_load_media_email: reuse send_job_status_mail

[Dominik Csapak <d.csapak@proxmox.com>]

On 3/11/21 9:09 AM, Thomas Lamprecht wrote:
> On 11.03.21 08:29, Dominik Csapak wrote:
>> so that we get consistent formatted mails from pbs
>> (e.g. html part and author)
>>
>> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
>> ---
>>   src/server/email_notifications.rs |  2 +-
>>   src/tape/changer/email.rs         | 13 ++-----------
>>   2 files changed, 3 insertions(+), 12 deletions(-)
>>
>> diff --git a/src/server/email_notifications.rs b/src/server/email_notifications.rs
>> index 306e84d0..a30c4d1a 100644
>> --- a/src/server/email_notifications.rs
>> +++ b/src/server/email_notifications.rs
>> @@ -205,7 +205,7 @@ lazy_static::lazy_static!{
>>       };
>>   }
>>   
>> -fn send_job_status_mail(
>> +pub fn send_job_status_mail(
> 
> we explicitly made that private, all other notfies happen in this place,
> send_updates_available
> 
> why does tape needs to be extra and have half here "send_tape_backup_status" and
> half in other modules?
> 
>>       email: &str,
>>       subject: &str,
>>       text: &str,
>> diff --git a/src/tape/changer/email.rs b/src/tape/changer/email.rs
>> index abd7ead8..5c6477cd 100644
>> --- a/src/tape/changer/email.rs
>> +++ b/src/tape/changer/email.rs
>> @@ -1,6 +1,6 @@
>>   use anyhow::Error;
>>   
>> -use proxmox::tools::email::sendmail;
>> +use crate::server::send_job_status_mail;
>>   
>>   /// Send email to a person to request a manual media change
>>   pub fn send_load_media_email(
> 
> why not just move this to server::email_notifications ?

yes makes sense, i'll send a v2

> 
>> @@ -23,14 +23,5 @@ pub fn send_load_media_email(
>>       text.push_str(&format!("Drive: {}\n", drive));
>>       text.push_str(&format!("Media: {}\n", label_text));
>>   
>> -    sendmail(
>> -        &[to],
>> -        &subject,
>> -        Some(&text),
>> -        None,
>> -        None,
>> -        None,
>> -    )?;
>> -
>> -    Ok(())
>> +    send_job_status_mail(to, &subject, &text)
>>   }
>>
>