From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with UTF8SMTPS id 2A66674907 for ; Wed, 2 Jun 2021 13:01:46 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with UTF8SMTP id 1AEAFA31E for ; Wed, 2 Jun 2021 13:01:46 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [94.136.29.106]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with UTF8SMTPS id EB619A30C for ; Wed, 2 Jun 2021 13:01:41 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with UTF8SMTP id C029E436FB for ; Wed, 2 Jun 2021 13:01:41 +0200 (CEST) Message-ID: <63cdedd8-5992-5bd2-e0d4-3e7dbf406eab@proxmox.com> Date: Wed, 2 Jun 2021 13:01:40 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:89.0) Gecko/20100101 Thunderbird/89.0 Content-Language: en-US To: pbs-devel@lists.proxmox.com References: <20210428102518.1578345-1-h.laimer@proxmox.com> From: Dominik Csapak In-Reply-To: <20210428102518.1578345-1-h.laimer@proxmox.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.354 Adjusted score from AWL reputation of From: address KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment NICE_REPLY_A -0.613 Looks like a legit reply (A) SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [datastore.rs] Subject: Re: [pbs-devel] [PATCH proxmox-backup] api2: add keep-job-configs flag to datastore remove endpoint X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 02 Jun 2021 11:01:46 -0000 high level comments: this changes the current behaviour of that call, so we should document this in the release notes/changelog explicitely else some users might wonder where their jobs are ... was probably not on your radar, but tape backup jobs would also fit in here remaining comments inline On 4/28/21 12:25, Hannes Laimer wrote: > Signed-off-by: Hannes Laimer > --- > Adds a flag that controls whether related sync and verify-jobs should > be kept or deleted. If not set or set to false, job configs will be removed > if set to true, job config files will be left untouched and may be used > in a new datastore with the same name. > > src/api2/config/datastore.rs | 30 +++++++++++++++++++++++++++--- > 1 file changed, 27 insertions(+), 3 deletions(-) > > diff --git a/src/api2/config/datastore.rs b/src/api2/config/datastore.rs > index 6ca98b53..f9794e7f 100644 > --- a/src/api2/config/datastore.rs > +++ b/src/api2/config/datastore.rs > @@ -1,13 +1,15 @@ > use std::path::PathBuf; > > use anyhow::{bail, Error}; > -use serde_json::Value; > +use serde_json::{Value, json}; > use ::serde::{Deserialize, Serialize}; > > use proxmox::api::{api, Router, RpcEnvironment, Permission}; > use proxmox::api::schema::parse_property_string; > use proxmox::tools::fs::open_file_locked; > > +use crate::api2::config::sync::{list_sync_jobs, delete_sync_job}; > +use crate::api2::config::verify::{list_verification_jobs, delete_verification_job}; > use crate::api2::types::*; > use crate::backup::*; > use crate::config::cached_user_info::CachedUserInfo; > @@ -392,6 +394,12 @@ pub fn update_datastore( > name: { > schema: DATASTORE_SCHEMA, > }, > + "keep-job-configs": { > + description: "If enabled, the job configurations related to this datastore will be kept.", > + type: bool, > + optional: true, > + default: false, > + }, > digest: { > optional: true, > schema: PROXMOX_CONFIG_DIGEST_SCHEMA, > @@ -403,8 +411,12 @@ pub fn update_datastore( > }, > )] > /// Remove a datastore configuration. > -pub fn delete_datastore(name: String, digest: Option) -> Result<(), Error> { > - > +pub fn delete_datastore( > + name: String, > + keep_job_configs: Option, if the api schema has a 'default', you can omit the 'Option' and it will be magically set to the default. this makes the code below much nicer since you can simply do if !keep_job_configs { ... } > + digest: Option, > + rpcenv: &mut dyn RpcEnvironment, > +) -> Result<(), Error> { > let _lock = open_file_locked(datastore::DATASTORE_CFG_LOCKFILE, std::time::Duration::new(10, 0), true)?; > > let (mut config, expected_digest) = datastore::config()?; > @@ -419,6 +431,18 @@ pub fn delete_datastore(name: String, digest: Option) -> Result<(), Erro > None => bail!("datastore '{}' does not exist.", name), > } > > + match keep_job_configs { > + Some(true) => {} > + _ => { // not set or set to false > + for job_config in list_verification_jobs(json!({ "store": name }), rpcenv)? { > + delete_verification_job(job_config.id, None, rpcenv)? > + } > + for job_config in list_sync_jobs(json!({ "store": name }), rpcenv)? { > + delete_sync_job(job_config.id, None, rpcenv)? > + } is there some other patch missing? you call 'list_verification_jobs' with a parameter, but that api call does not do anything with them? afaics, this would remove all sync and verification jobs or did you mean to call crate::api2::admin::sync::list_sync_jobs ? note the difference of 'api2::admin::' and 'api2::config' > + } > + } > + > datastore::save_config(&config)?; > > // ignore errors >