public inbox for pve-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pve-devel] [PATCH manager 1/2] fix #5067: vzdump: include total backup size in notification
@ 2023-12-14 14:11 Lukas Wagner
  2023-12-14 14:11 ` [pve-devel] [PATCH manager 2/2] vzdump: make helper functions for sending notifications private Lukas Wagner
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Lukas Wagner @ 2023-12-14 14:11 UTC (permalink / raw)
  To: pve-devel

The old backup job notification mails from before the notification
system overhaul included the total time as well as the total size.

The total size was missing from the new, template-based backup report,
thus we add it back in this commit.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 PVE/VZDump.pm | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
index 4185ed62..91f60418 100644
--- a/PVE/VZDump.pm
+++ b/PVE/VZDump.pm
@@ -414,15 +414,17 @@ sub sanitize_task_list {
     }
 }
 
-sub count_failed_tasks {
+my sub aggregate_task_statistics {
     my ($tasklist) = @_;
 
     my $error_count = 0;
+    my $total_size = 0;
     for my $task (@$tasklist) {
 	$error_count++ if $task->{state} ne 'ok';
+	$total_size += $task->{size} if $task->{state} eq 'ok';
     }
 
-    return $error_count;
+    return $error_count, $total_size;
 }
 
 sub get_hostname {
@@ -437,9 +439,10 @@ my $body_template = <<EOT;
 {{error-message}}
 {{heading-1 "Details"}}
 {{table guest-table}}
-
+{{#verbatim}}
 Total running time: {{duration total-time}}
-
+Total size: {{human-bytes total-size}}
+{{/verbatim}}
 {{heading-1 "Logs"}}
 {{verbatim-monospaced logs}}
 EOT
@@ -456,7 +459,7 @@ sub send_notification {
     my $mode = $opts->{"notification-mode"} // 'auto';
 
     sanitize_task_list($tasklist);
-    my $error_count = count_failed_tasks($tasklist);
+    my ($error_count, $total_size) = aggregate_task_statistics($tasklist);
 
     my $failed = ($error_count || $err);
 
@@ -486,12 +489,13 @@ sub send_notification {
     my $hostname = get_hostname();
 
     my $notification_props = {
-	"hostname"      => $hostname,
+	"hostname" => $hostname,
 	"error-message" => $err,
-	"guest-table"   => build_guest_table($tasklist),
-	"logs"          => $text_log_part,
-	"status-text"   => $status_text,
-	"total-time"    => $total_time,
+	"guest-table" => build_guest_table($tasklist),
+	"logs" => $text_log_part,
+	"status-text" => $status_text,
+	"total-time" => $total_time,
+	"total-size" => $total_size,
     };
 
     my $fields = {
-- 
2.39.2





^ permalink raw reply	[flat|nested] 4+ messages in thread

* [pve-devel] [PATCH manager 2/2] vzdump: make helper functions for sending notifications private
  2023-12-14 14:11 [pve-devel] [PATCH manager 1/2] fix #5067: vzdump: include total backup size in notification Lukas Wagner
@ 2023-12-14 14:11 ` Lukas Wagner
  2024-01-08 10:44 ` [pve-devel] [PATCH manager 1/2] fix #5067: vzdump: include total backup size in notification Lukas Wagner
  2024-01-29  9:38 ` [pve-devel] applied-series: " Fiona Ebner
  2 siblings, 0 replies; 4+ messages in thread
From: Lukas Wagner @ 2023-12-14 14:11 UTC (permalink / raw)
  To: pve-devel

The helpers were split out from the original 'sendmail' function when
migrating to the new notification system. They are not needed anywhere
else and can thus be private.

Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
---
 PVE/VZDump.pm | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
index 91f60418..009eb369 100644
--- a/PVE/VZDump.pm
+++ b/PVE/VZDump.pm
@@ -318,7 +318,7 @@ sub read_vzdump_defaults {
     return $res;
 }
 
-sub read_backup_task_logs {
+my sub read_backup_task_logs {
     my ($task_list) = @_;
 
     my $task_logs = "";
@@ -345,7 +345,7 @@ sub read_backup_task_logs {
     return $task_logs;
 }
 
-sub build_guest_table {
+my sub build_guest_table {
     my ($task_list) = @_;
 
     my $table = {
@@ -399,7 +399,7 @@ sub build_guest_table {
     return $table;
 }
 
-sub sanitize_task_list {
+my sub sanitize_task_list {
     my ($task_list) = @_;
     for my $task (@$task_list) {
 	chomp $task->{msg} if $task->{msg};
@@ -427,7 +427,7 @@ my sub aggregate_task_statistics {
     return $error_count, $total_size;
 }
 
-sub get_hostname {
+my sub get_hostname {
     my $hostname = `hostname -f` || PVE::INotify::nodename();
     chomp $hostname;
     return $hostname;
-- 
2.39.2





^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [pve-devel] [PATCH manager 1/2] fix #5067: vzdump: include total backup size in notification
  2023-12-14 14:11 [pve-devel] [PATCH manager 1/2] fix #5067: vzdump: include total backup size in notification Lukas Wagner
  2023-12-14 14:11 ` [pve-devel] [PATCH manager 2/2] vzdump: make helper functions for sending notifications private Lukas Wagner
@ 2024-01-08 10:44 ` Lukas Wagner
  2024-01-29  9:38 ` [pve-devel] applied-series: " Fiona Ebner
  2 siblings, 0 replies; 4+ messages in thread
From: Lukas Wagner @ 2024-01-08 10:44 UTC (permalink / raw)
  To: pve-devel

ping

On 12/14/23 15:11, Lukas Wagner wrote:
> The old backup job notification mails from before the notification
> system overhaul included the total time as well as the total size.
> 
> The total size was missing from the new, template-based backup report,
> thus we add it back in this commit.
> 
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---
>   PVE/VZDump.pm | 24 ++++++++++++++----------
>   1 file changed, 14 insertions(+), 10 deletions(-)
> 
> diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
> index 4185ed62..91f60418 100644
> --- a/PVE/VZDump.pm
> +++ b/PVE/VZDump.pm
> @@ -414,15 +414,17 @@ sub sanitize_task_list {
>       }
>   }
>   
> -sub count_failed_tasks {
> +my sub aggregate_task_statistics {
>       my ($tasklist) = @_;
>   
>       my $error_count = 0;
> +    my $total_size = 0;
>       for my $task (@$tasklist) {
>   	$error_count++ if $task->{state} ne 'ok';
> +	$total_size += $task->{size} if $task->{state} eq 'ok';
>       }
>   
> -    return $error_count;
> +    return $error_count, $total_size;
>   }
>   
>   sub get_hostname {
> @@ -437,9 +439,10 @@ my $body_template = <<EOT;
>   {{error-message}}
>   {{heading-1 "Details"}}
>   {{table guest-table}}
> -
> +{{#verbatim}}
>   Total running time: {{duration total-time}}
> -
> +Total size: {{human-bytes total-size}}
> +{{/verbatim}}
>   {{heading-1 "Logs"}}
>   {{verbatim-monospaced logs}}
>   EOT
> @@ -456,7 +459,7 @@ sub send_notification {
>       my $mode = $opts->{"notification-mode"} // 'auto';
>   
>       sanitize_task_list($tasklist);
> -    my $error_count = count_failed_tasks($tasklist);
> +    my ($error_count, $total_size) = aggregate_task_statistics($tasklist);
>   
>       my $failed = ($error_count || $err);
>   
> @@ -486,12 +489,13 @@ sub send_notification {
>       my $hostname = get_hostname();
>   
>       my $notification_props = {
> -	"hostname"      => $hostname,
> +	"hostname" => $hostname,
>   	"error-message" => $err,
> -	"guest-table"   => build_guest_table($tasklist),
> -	"logs"          => $text_log_part,
> -	"status-text"   => $status_text,
> -	"total-time"    => $total_time,
> +	"guest-table" => build_guest_table($tasklist),
> +	"logs" => $text_log_part,
> +	"status-text" => $status_text,
> +	"total-time" => $total_time,
> +	"total-size" => $total_size,
>       };
>   
>       my $fields = {

-- 
- Lukas




^ permalink raw reply	[flat|nested] 4+ messages in thread

* [pve-devel] applied-series: [PATCH manager 1/2] fix #5067: vzdump: include total backup size in notification
  2023-12-14 14:11 [pve-devel] [PATCH manager 1/2] fix #5067: vzdump: include total backup size in notification Lukas Wagner
  2023-12-14 14:11 ` [pve-devel] [PATCH manager 2/2] vzdump: make helper functions for sending notifications private Lukas Wagner
  2024-01-08 10:44 ` [pve-devel] [PATCH manager 1/2] fix #5067: vzdump: include total backup size in notification Lukas Wagner
@ 2024-01-29  9:38 ` Fiona Ebner
  2 siblings, 0 replies; 4+ messages in thread
From: Fiona Ebner @ 2024-01-29  9:38 UTC (permalink / raw)
  To: Proxmox VE development discussion, Lukas Wagner

Am 14.12.23 um 15:11 schrieb Lukas Wagner:
> The old backup job notification mails from before the notification
> system overhaul included the total time as well as the total size.
> 
> The total size was missing from the new, template-based backup report,
> thus we add it back in this commit.
> 
> Signed-off-by: Lukas Wagner <l.wagner@proxmox.com>
> ---

applied series, thanks!

>  PVE/VZDump.pm | 24 ++++++++++++++----------
>  1 file changed, 14 insertions(+), 10 deletions(-)
> 
> diff --git a/PVE/VZDump.pm b/PVE/VZDump.pm
> index 4185ed62..91f60418 100644
> --- a/PVE/VZDump.pm
> +++ b/PVE/VZDump.pm
> @@ -414,15 +414,17 @@ sub sanitize_task_list {
>      }
>  }
>  
> -sub count_failed_tasks {
> +my sub aggregate_task_statistics {
>      my ($tasklist) = @_;
>  
>      my $error_count = 0;
> +    my $total_size = 0;
>      for my $task (@$tasklist) {
>  	$error_count++ if $task->{state} ne 'ok';
> +	$total_size += $task->{size} if $task->{state} eq 'ok';
>      }
>  
> -    return $error_count;
> +    return $error_count, $total_size;
>  }

With a small style fix to use parentheses for the list return here.




^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2024-01-29  9:38 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-14 14:11 [pve-devel] [PATCH manager 1/2] fix #5067: vzdump: include total backup size in notification Lukas Wagner
2023-12-14 14:11 ` [pve-devel] [PATCH manager 2/2] vzdump: make helper functions for sending notifications private Lukas Wagner
2024-01-08 10:44 ` [pve-devel] [PATCH manager 1/2] fix #5067: vzdump: include total backup size in notification Lukas Wagner
2024-01-29  9:38 ` [pve-devel] applied-series: " Fiona Ebner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal