Re: [pbs-devel] [PATCH v1 proxmox-backup 00/14] add job based verify scheduling

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

Some high level comments:

* patches need some work on rebase, since there were some code changes
* we sadly cannot simply remove the 'verify-schedule' from the
   datastore configs, as we already have packages out there that support
   it and it breaks all datastore config parsing

we could fix that either by
  - leaving the schedule in place and 'magically' show it as a verifyjob
  - have some script that runs in postinst that rewrites the verify
    schedules as jobs
  - maybe some other method?

* it would be very nice to see which backups were considered for
   verification, but skipped by its verify status, else the user
   clicks on verification and only gets: Ok
   but with no progress or indication what actually happened

* for now i think it's okay to add a separate panel, though
   thomas would prefer to somehow merge the syncjob/verifyjob
   (and maybe also garbage_collection/prune) into a single
   "Job/Schedule" overview

the patches itself look mostly ok though and work
as expected (after correctly rebasing)

i'll also write some comments to the patches directly

On 9/25/20 10:43 AM, Hannes Laimer wrote:
> Replaces the first implementation of scheduled verification with a new
> job-based version with additional options that may be specified through
> the web ui.
> 
> Options available for verification jobs:
>   * schedule when to run the job
>   * set datastore on which the job should run
>   * set a number of days after which a verification becomes "outdated"
>      empty => verifications are valid forever
>   * specify if already successfuly verified snapshots should be verified
>      again even if they're not outdated(failed ones will always be done)
> 
> 
> Hannes Laimer (14):
>    add two new schemas for verify jobs
>    add verify job config
>    api2: add verify job config endpoint
>    add do_verification_job function to verify.rs
>    api2: add verify job admin endpoint
>    add scheduling for verify jobs
>    set a diffrent worker_type based on what is going to be
>      verified(snapshot,group,ds)
>    ui: add verify job view
>    ui: add verify job edit window
>    ui: add task descriptions for the different types of
>      verify(job,snapshot,group,ds)
>    remove verify_schedule field from DatastoreConfig
>    remove verify_schedule field from datastore config endpoint
>    remove verify-schedule field from DataStoreEdit and DataStoreConfig
>    remove old verification scheduling from proxmox-backup-proxy.rs
> 
>   src/api2/admin.rs               |   4 +-
>   src/api2/admin/datastore.rs     |   5 +-
>   src/api2/admin/verify.rs        | 107 +++++++++++++
>   src/api2/config.rs              |   2 +
>   src/api2/config/datastore.rs    |  13 --
>   src/api2/config/verify.rs       | 272 +++++++++++++++++++++++++++++++
>   src/api2/types/mod.rs           |  10 ++
>   src/backup/verify.rs            |  91 ++++++++++-
>   src/bin/proxmox-backup-proxy.rs | 103 +++++-------
>   src/config.rs                   |   1 +
>   src/config/datastore.rs         |   6 -
>   src/config/verify.rs            | 186 ++++++++++++++++++++++
>   www/Makefile                    |   2 +
>   www/NavigationTree.js           |   6 +
>   www/Utils.js                    |   7 +-
>   www/config/DataStoreConfig.js   |   5 +-
>   www/config/VerifyView.js        | 273 ++++++++++++++++++++++++++++++++
>   www/window/DataStoreEdit.js     |   9 --
>   www/window/VerifyJobEdit.js     |  89 +++++++++++
>   19 files changed, 1089 insertions(+), 102 deletions(-)
>   create mode 100644 src/api2/admin/verify.rs
>   create mode 100644 src/api2/config/verify.rs
>   create mode 100644 src/config/verify.rs
>   create mode 100644 www/config/VerifyView.js
>   create mode 100644 www/window/VerifyJobEdit.js
>