Re: [pbs-devel] [RFC proxmox-backup] PruneJobConfig: make "schedule" optional

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

Am 16/11/2023 um 14:19 schrieb Dietmar Maurer:
> The API macro already mark this property as optional:
> 
>   schedule: {
>             schema: PRUNE_SCHEDULE_SCHEMA,
>             optional: true,
>   },
> 
> So the correct representation is Option<String> (not String).
> 
> This is now consistent with other job types.
> ---
> 
> Honestly, I have no idea if it was indented to make schedule non-optional?

The whole mix of behavior is due to originally having only one prune
job per datastore, directly encoded in the datastore config.

There it could be disabled by setting the prune schedule to None, see
the screenshot from 1.x times:

https://pbs.proxmox.com/docs-1/_images/pbs-gui-datastore-prunegc.png

Prune jobs got added to allow users to make better use of namespaces,
as there one might want to have different retention per namespace, and
we tried to stay backward compatible with the existing job from
datastore.cfg, but also use our "modern" disable flag like we added
on various places (IIRC one of my first patches was to add that feature
to PVE backup jobs almost a decade ago...).

> Also, why have prune jobs a disable property, while other jobs types
> do not have it?

Because prune jobs tried to use our modern feature set, which includes
a enable/disable flag so users can easily switch a job of without removing
it as a whole, or delete the job's schedule, which can be a nuisance to
restore again, especially if it was a more complex one.

In other words, sync and verify jobs should gain the disable flag for
feature parity, and to provide a slightly more obvious way to (temporarily)
disable a job without loss of (schedule) information.
 
> Also, update_to_prune_jobs_config() looks wrong now because it remove jobs
> without a schedule. Was this really indendet?

Without looking to closely into it: That seems indeed rather odd and
is likely a bug. I did not check your change to closely (sorry, a bit
overloaded with PVE stuff currently) but did you fix that here too?