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

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

[Dominik Csapak]

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(
     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(
@@ -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)
 }
-- 
2.20.1

[pbs-devel] [PATCH proxmox-backup 2/2] server/worker_task: improve endtime for unknown tasks

[Dominik Csapak]

instead of always using the starttime, use the last timestamp from the log
this way, one can see when the task was aborted without having to read
the log

Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
---
 src/server/worker_task.rs | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/server/worker_task.rs b/src/server/worker_task.rs
index 1e8e009f..6c5456c9 100644
--- a/src/server/worker_task.rs
+++ b/src/server/worker_task.rs
@@ -207,6 +207,8 @@ pub fn upid_read_status(upid: &UPID) -> Result<TaskState, Error> {
     let mut iter = last_line.splitn(2, ": ");
     if let Some(time_str) = iter.next() {
         if let Ok(endtime) = proxmox::tools::time::parse_rfc3339(time_str) {
+            // set the endtime even if we cannot parse the state
+            status = TaskState::Unknown { endtime };
             if let Some(rest) = iter.next().and_then(|rest| rest.strip_prefix("TASK ")) {
                 if let Ok(state) = TaskState::from_endtime_and_message(endtime, rest) {
                     status = state;
-- 
2.20.1

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

[Thomas Lamprecht]

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 ?

> @@ -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)
>  }
> 

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

[Dominik Csapak]

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)
>>   }
>>
>