From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) by lore.proxmox.com (Postfix) with ESMTPS id 7DF521FF394 for ; Mon, 3 Jun 2024 10:03:10 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id F083B1C9ED; Mon, 3 Jun 2024 10:03:38 +0200 (CEST) Date: Mon, 3 Jun 2024 10:03:05 +0200 From: Wolfgang Bumiller To: Gabriel Goller Message-ID: <6m7gwbhtuvui6gvo5m5vxha72qbmboiiiiuesl7x362wrl6rna@3gnprybqp37f> References: <20240426123720.110120-1-g.goller@proxmox.com> <20240426123720.110120-3-g.goller@proxmox.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20240426123720.110120-3-g.goller@proxmox.com> X-SPAM-LEVEL: Spam detection results: 0 AWL 0.090 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record T_SCC_BODY_TEXT_LINE -0.01 - Subject: Re: [pbs-devel] [PATCH proxmox-backup v6 2/2] close #4763: client: add command to forget backup group 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: , Reply-To: Proxmox Backup Server development discussion Cc: pbs-devel@lists.proxmox.com Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pbs-devel-bounces@lists.proxmox.com Sender: "pbs-devel" style nit On Fri, Apr 26, 2024 at 02:37:16PM GMT, Gabriel Goller wrote: > Add the command `proxmox-backup-client group forget ` so > that we can forget (delete) whole groups with all the containing > snapshots. > To avoid printing full datastore paths (which are in the error messages) > we filter out the most common one (group not found) and rephrase it. > > Signed-off-by: Gabriel Goller > --- > proxmox-backup-client/src/group.rs | 88 ++++++++++++++++++++++++++++++ > proxmox-backup-client/src/main.rs | 3 + > 2 files changed, 91 insertions(+) > create mode 100644 proxmox-backup-client/src/group.rs > > diff --git a/proxmox-backup-client/src/group.rs b/proxmox-backup-client/src/group.rs > new file mode 100644 > index 00000000..8a2a4e1e > --- /dev/null > +++ b/proxmox-backup-client/src/group.rs > @@ -0,0 +1,88 @@ > +use anyhow::{bail, Error}; > +use serde_json::Value; > + > +use proxmox_router::cli::{CliCommand, CliCommandMap, Confirmation}; > +use proxmox_schema::api; > + > +use crate::{ > + complete_backup_group, complete_namespace, complete_repository, merge_group_into, > + REPO_URL_SCHEMA, > +}; > +use pbs_api_types::{BackupGroup, BackupNamespace}; > +use pbs_client::tools::{connect, extract_repository_from_value}; > + > +pub fn group_mgmt_cli() -> CliCommandMap { > + CliCommandMap::new().insert( > + "forget", > + CliCommand::new(&API_METHOD_FORGET_GROUP) > + .arg_param(&["group"]) > + .completion_cb("ns", complete_namespace) > + .completion_cb("repository", complete_repository) > + .completion_cb("group", complete_backup_group), > + ) > +} > + > +#[api( > + input: { > + properties: { > + group: { > + type: String, > + description: "Backup group", > + }, > + repository: { > + schema: REPO_URL_SCHEMA, > + optional: true, > + }, > + ns: { > + type: BackupNamespace, > + optional: true, > + }, > + } > + } > +)] > +/// Forget (remove) backup snapshots. > +async fn forget_group(group: String, param: Value) -> Result { We have this a lot, but we should stop doing this. API methods which don't return anything can juts use `Result<(), Error>`. > + let backup_group: BackupGroup = group.parse()?; > + let repo = extract_repository_from_value(¶m)?; > + let client = connect(&repo)?; > + > + let mut api_params = param; > + merge_group_into(api_params.as_object_mut().unwrap(), backup_group.clone()); > + > + let path = format!("api2/json/admin/datastore/{}/snapshots", repo.store()); > + let result = client.get(&path, Some(api_params.clone())).await?; > + let snapshots = result["data"].as_array().unwrap().len(); > + > + let confirmation = Confirmation::query_with_default( > + format!( > + "Delete group \"{}\" with {} snapshot(s)?", > + backup_group, snapshots > + ) > + .as_str(), > + Confirmation::No, > + )?; > + if confirmation.is_yes() { > + let path = format!("api2/json/admin/datastore/{}/groups", repo.store()); > + if let Err(err) = client.delete(&path, Some(api_params)).await { > + // "ENOENT: No such file or directory" is part of the error returned when the group > + // has not been found. The full error contains the full datastore path and we would > + // like to avoid printing that to the console. Checking if it exists before deleting > + // the group doesn't work because we currently do not differentiate between an empty > + // and a nonexistent group. This would make it impossible to remove empty groups. > + if err > + .root_cause() > + .to_string() > + .contains("ENOENT: No such file or directory") > + { > + bail!("Unable to find backup group!"); > + } else { > + bail!(err); > + } > + } > + println!("Successfully deleted group!"); > + } else { > + println!("Abort."); > + } > + > + Ok(Value::Null) And use `Ok(())` here. > +} _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel