From: Dietmar Maurer <dietmar@proxmox.com>
To: pbs-devel@lists.proxmox.com
Subject: [pbs-devel] [RFC proxmox-backup] PruneJobConfig: make "schedule" optional
Date: Thu, 16 Nov 2023 14:19:04 +0100 [thread overview]
Message-ID: <20231116131904.2415371-1-dietmar@proxmox.com> (raw)
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?
Also, why have prune jobs a disable property, while other jobs types
do not have it?
Also, update_to_prune_jobs_config() looks wrong now because it remove jobs
without a schedule. Was this really indendet?
pbs-api-types/src/jobs.rs | 3 ++-
src/api2/admin/prune.rs | 2 +-
src/api2/config/prune.rs | 12 ++++++++----
src/bin/proxmox-backup-proxy.rs | 9 +++++++--
src/bin/proxmox_backup_manager/prune.rs | 2 +-
5 files changed, 19 insertions(+), 9 deletions(-)
diff --git a/pbs-api-types/src/jobs.rs b/pbs-api-types/src/jobs.rs
index f3e64ee4..1fb74381 100644
--- a/pbs-api-types/src/jobs.rs
+++ b/pbs-api-types/src/jobs.rs
@@ -712,7 +712,8 @@ pub struct PruneJobConfig {
#[updater(serde(skip_serializing_if = "Option::is_none"))]
pub disable: bool,
- pub schedule: String,
+ #[serde(skip_serializing_if = "Option::is_none")]
+ pub schedule: Option<String>,
#[serde(skip_serializing_if = "Option::is_none")]
pub comment: Option<String>,
diff --git a/src/api2/admin/prune.rs b/src/api2/admin/prune.rs
index a5ebf297..85dab54f 100644
--- a/src/api2/admin/prune.rs
+++ b/src/api2/admin/prune.rs
@@ -76,7 +76,7 @@ pub fn list_prune_jobs(
let last_state = JobState::load("prunejob", &job.id)
.map_err(|err| format_err!("could not open statefile for {}: {}", &job.id, err))?;
- let mut status = compute_schedule_status(&last_state, Some(&job.schedule))?;
+ let mut status = compute_schedule_status(&last_state, job.schedule.as_deref())?;
if job.disable {
status.next_run = None;
}
diff --git a/src/api2/config/prune.rs b/src/api2/config/prune.rs
index 6f391722..4679431d 100644
--- a/src/api2/config/prune.rs
+++ b/src/api2/config/prune.rs
@@ -157,6 +157,8 @@ pub enum DeletableProperty {
KeepMonthly,
/// Delete number of yearly backups to keep.
KeepYearly,
+ /// Delete the job schedule.
+ Schedule,
}
#[api(
@@ -248,6 +250,9 @@ pub fn update_prune_job(
DeletableProperty::KeepYearly => {
data.options.keep.keep_yearly = None;
}
+ DeletableProperty::Schedule => {
+ data.schedule = None;
+ }
}
}
}
@@ -268,10 +273,9 @@ pub fn update_prune_job(
user_info.check_privs(&auth_id, &data.acl_path(), PRIV_DATASTORE_MODIFY, true)?;
}
- let mut schedule_changed = false;
- if let Some(schedule) = update.schedule {
- schedule_changed = data.schedule != schedule;
- data.schedule = schedule;
+ let schedule_changed = data.schedule != update.schedule;
+ if update.schedule.is_some() {
+ data.schedule = update.schedule;
}
if let Some(max_depth) = update.options.max_depth {
diff --git a/src/bin/proxmox-backup-proxy.rs b/src/bin/proxmox-backup-proxy.rs
index f38a02bd..e7a124bc 100644
--- a/src/bin/proxmox-backup-proxy.rs
+++ b/src/bin/proxmox-backup-proxy.rs
@@ -556,9 +556,14 @@ async fn schedule_datastore_prune_jobs() {
continue; // no 'keep' values set, keep all
}
+ let event_str = match job_config.schedule {
+ Some(ref event_str) => event_str.clone(),
+ None => continue,
+ };
+
let worker_type = "prunejob";
let auth_id = Authid::root_auth_id().clone();
- if check_schedule(worker_type, &job_config.schedule, &job_id) {
+ if check_schedule(worker_type, &event_str, &job_id) {
let job = match Job::new(worker_type, &job_id) {
Ok(job) => job,
Err(_) => continue, // could not get lock
@@ -568,7 +573,7 @@ async fn schedule_datastore_prune_jobs() {
job_config.options,
job_config.store,
&auth_id,
- Some(job_config.schedule),
+ Some(event_str),
) {
eprintln!("unable to start datastore prune job {job_id} - {err}");
}
diff --git a/src/bin/proxmox_backup_manager/prune.rs b/src/bin/proxmox_backup_manager/prune.rs
index 923eb6f5..7738f661 100644
--- a/src/bin/proxmox_backup_manager/prune.rs
+++ b/src/bin/proxmox_backup_manager/prune.rs
@@ -245,7 +245,7 @@ pub(crate) fn update_to_prune_jobs_config() -> Result<(), Error> {
store: store.clone(),
disable: false,
comment: None,
- schedule,
+ schedule: Some(schedule),
options,
};
--
2.39.2
next reply other threads:[~2023-11-16 13:19 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-11-16 13:19 Dietmar Maurer [this message]
2023-11-16 13:58 ` Thomas Lamprecht
2023-11-16 14:07 ` Dietmar Maurer
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20231116131904.2415371-1-dietmar@proxmox.com \
--to=dietmar@proxmox.com \
--cc=pbs-devel@lists.proxmox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox