all lists on lists.proxmox.com
 help / color / mirror / Atom feed
* [pbs-devel] [PATCH proxmox-backup] fix #6624: ui: improve task log output for protected entry
@ 2025-08-22 16:45 Shan Shaji
  2025-08-25  9:47 ` Wolfgang Bumiller
  0 siblings, 1 reply; 4+ messages in thread
From: Shan Shaji @ 2025-08-22 16:45 UTC (permalink / raw)
  To: pbs-devel; +Cc: Shan Shaji

Using 'Prune All' with 'Dry Run' gives incomplete task-log
output for a 'protected' entry. To fix this, add an additional
check to return the string 'would keep', since a protected
entry is always kept.

Signed-off-by: Shan Shaji <s.shaji@proxmox.com>
---
 src/server/prune_job.rs | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/server/prune_job.rs b/src/server/prune_job.rs
index 1c86647a..6108d32d 100644
--- a/src/server/prune_job.rs
+++ b/src/server/prune_job.rs
@@ -69,7 +69,15 @@ pub fn prune_datastore(
             let keep = keep_all || mark.keep();
             info!(
                 "{}{mark} {}/{}/{}",
-                if dry_run { "would " } else { "" },
+                if dry_run {
+                    if mark.protected() {
+                        "would keep "
+                    } else {
+                        "would "
+                    }
+                } else {
+                    ""
+                },
                 group.backup_type(),
                 group.backup_id(),
                 info.backup_dir.backup_time_string()
-- 
2.47.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


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

* Re: [pbs-devel] [PATCH proxmox-backup] fix #6624: ui: improve task log output for protected entry
  2025-08-22 16:45 [pbs-devel] [PATCH proxmox-backup] fix #6624: ui: improve task log output for protected entry Shan Shaji
@ 2025-08-25  9:47 ` Wolfgang Bumiller
  2025-08-25 10:47   ` Shan Shaji
  2025-08-25 13:23   ` Shan Shaji
  0 siblings, 2 replies; 4+ messages in thread
From: Wolfgang Bumiller @ 2025-08-25  9:47 UTC (permalink / raw)
  To: Shan Shaji; +Cc: pbs-devel

On Fri, Aug 22, 2025 at 06:45:48PM +0200, Shan Shaji wrote:
> Using 'Prune All' with 'Dry Run' gives incomplete task-log
> output for a 'protected' entry. To fix this, add an additional
> check to return the string 'would keep', since a protected
> entry is always kept.
> 
> Signed-off-by: Shan Shaji <s.shaji@proxmox.com>
> ---
>  src/server/prune_job.rs | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/src/server/prune_job.rs b/src/server/prune_job.rs
> index 1c86647a..6108d32d 100644
> --- a/src/server/prune_job.rs
> +++ b/src/server/prune_job.rs
> @@ -69,7 +69,15 @@ pub fn prune_datastore(
>              let keep = keep_all || mark.keep();
>              info!(
>                  "{}{mark} {}/{}/{}",
> -                if dry_run { "would " } else { "" },
> +                if dry_run {
> +                    if mark.protected() {
> +                        "would keep "
> +                    } else {
> +                        "would "
> +                    }
> +                } else {
> +                    ""
> +                },

^ Given how large this is - if we want to make sure the sentence
structure is "nice", should we not instead add a:

    mark = match mark {
        PruneMark::Protected => "keep protected",
        PruneMark::Keep => "keep",
        PruneMark::KeepPartial => "keep partial",
        PruneMark::Remove => "remove",
    },

Because `KeepPartial` would otherwise have a hyphen in it, which I'd
find kind of awkward.

>                  group.backup_type(),
>                  group.backup_id(),
>                  info.backup_dir.backup_time_string()
> -- 
> 2.47.2


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


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

* Re: [pbs-devel] [PATCH proxmox-backup] fix #6624: ui: improve task log output for protected entry
  2025-08-25  9:47 ` Wolfgang Bumiller
@ 2025-08-25 10:47   ` Shan Shaji
  2025-08-25 13:23   ` Shan Shaji
  1 sibling, 0 replies; 4+ messages in thread
From: Shan Shaji @ 2025-08-25 10:47 UTC (permalink / raw)
  To: Wolfgang Bumiller; +Cc: pbs-devel

Hi, Thank you so much for the review.

That makes sense, i will update it accordingly. 

On Mon Aug 25, 2025 at 11:47 AM CEST, Wolfgang Bumiller wrote:
> On Fri, Aug 22, 2025 at 06:45:48PM +0200, Shan Shaji wrote:
>> Using 'Prune All' with 'Dry Run' gives incomplete task-log
>> output for a 'protected' entry. To fix this, add an additional
>> check to return the string 'would keep', since a protected
>> entry is always kept.
>> 
>> Signed-off-by: Shan Shaji <s.shaji@proxmox.com>
>> ---
>>  src/server/prune_job.rs | 10 +++++++++-
>>  1 file changed, 9 insertions(+), 1 deletion(-)
>> 
>> diff --git a/src/server/prune_job.rs b/src/server/prune_job.rs
>> index 1c86647a..6108d32d 100644
>> --- a/src/server/prune_job.rs
>> +++ b/src/server/prune_job.rs
>> @@ -69,7 +69,15 @@ pub fn prune_datastore(
>>              let keep = keep_all || mark.keep();
>>              info!(
>>                  "{}{mark} {}/{}/{}",
>> -                if dry_run { "would " } else { "" },
>> +                if dry_run {
>> +                    if mark.protected() {
>> +                        "would keep "
>> +                    } else {
>> +                        "would "
>> +                    }
>> +                } else {
>> +                    ""
>> +                },
>
> ^ Given how large this is - if we want to make sure the sentence
> structure is "nice", should we not instead add a:
>
>     mark = match mark {
>         PruneMark::Protected => "keep protected",
>         PruneMark::Keep => "keep",
>         PruneMark::KeepPartial => "keep partial",
>         PruneMark::Remove => "remove",
>     },
>
> Because `KeepPartial` would otherwise have a hyphen in it, which I'd
> find kind of awkward.
>
>>                  group.backup_type(),
>>                  group.backup_id(),
>>                  info.backup_dir.backup_time_string()
>> -- 
>> 2.47.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


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

* Re: [pbs-devel] [PATCH proxmox-backup] fix #6624: ui: improve task log output for protected entry
  2025-08-25  9:47 ` Wolfgang Bumiller
  2025-08-25 10:47   ` Shan Shaji
@ 2025-08-25 13:23   ` Shan Shaji
  1 sibling, 0 replies; 4+ messages in thread
From: Shan Shaji @ 2025-08-25 13:23 UTC (permalink / raw)
  To: Wolfgang Bumiller; +Cc: pbs-devel

Superseded by v2: https://lore.proxmox.com/pbs-devel/20250825132103.225192-1-s.shaji@proxmox.com/T/#u

On Mon Aug 25, 2025 at 11:47 AM CEST, Wolfgang Bumiller wrote:
> On Fri, Aug 22, 2025 at 06:45:48PM +0200, Shan Shaji wrote:
>> Using 'Prune All' with 'Dry Run' gives incomplete task-log
>> output for a 'protected' entry. To fix this, add an additional
>> check to return the string 'would keep', since a protected
>> entry is always kept.
>> 
>> Signed-off-by: Shan Shaji <s.shaji@proxmox.com>
>> ---
>>  src/server/prune_job.rs | 10 +++++++++-
>>  1 file changed, 9 insertions(+), 1 deletion(-)
>> 
>> diff --git a/src/server/prune_job.rs b/src/server/prune_job.rs
>> index 1c86647a..6108d32d 100644
>> --- a/src/server/prune_job.rs
>> +++ b/src/server/prune_job.rs
>> @@ -69,7 +69,15 @@ pub fn prune_datastore(
>>              let keep = keep_all || mark.keep();
>>              info!(
>>                  "{}{mark} {}/{}/{}",
>> -                if dry_run { "would " } else { "" },
>> +                if dry_run {
>> +                    if mark.protected() {
>> +                        "would keep "
>> +                    } else {
>> +                        "would "
>> +                    }
>> +                } else {
>> +                    ""
>> +                },
>
> ^ Given how large this is - if we want to make sure the sentence
> structure is "nice", should we not instead add a:
>
>     mark = match mark {
>         PruneMark::Protected => "keep protected",
>         PruneMark::Keep => "keep",
>         PruneMark::KeepPartial => "keep partial",
>         PruneMark::Remove => "remove",
>     },
>
> Because `KeepPartial` would otherwise have a hyphen in it, which I'd
> find kind of awkward.
>
>>                  group.backup_type(),
>>                  group.backup_id(),
>>                  info.backup_dir.backup_time_string()
>> -- 
>> 2.47.2



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel


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

end of thread, other threads:[~2025-08-25 13:23 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-08-22 16:45 [pbs-devel] [PATCH proxmox-backup] fix #6624: ui: improve task log output for protected entry Shan Shaji
2025-08-25  9:47 ` Wolfgang Bumiller
2025-08-25 10:47   ` Shan Shaji
2025-08-25 13:23   ` Shan Shaji

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal