[pbs-devel] [PATCH proxmox{, -backup} v6 0/2] close #4763: client: add command to forget backup group

[pbs-devel] [PATCH proxmox{, -backup} v6 0/2] close #4763: client: add command to forget backup group

[Gabriel Goller]

Add the command `proxmox-backup-client group forget <group>` so
that we can forget (delete) whole groups with all the containing
snapshots. As this is quite dangerous, we added a prompt, so the
user has to confirm the operation.

v6, thanks @Thomas:
 - add more generic confirmation helper drawn up by @Thomas
 - stricter yes/no matching with default

v5, thanks @Christian:
 - stricter input matching ($ in regex)
 - avoid printing full datastore path

v4:
 - removed localization

--version 3 has been skipped--

v2:
 - added localization

proxmox:

Gabriel Goller (1):
  router: cli: add confirmation helper

 proxmox-router/Cargo.toml     |   1 +
 proxmox-router/src/cli/mod.rs | 113 +++++++++++++++++++++++++++++++++-
 2 files changed, 113 insertions(+), 1 deletion(-)


proxmox-backup:

Gabriel Goller (1):
  close #4763: client: add command to forget backup group

 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


Summary over all repositories:
  4 files changed, 204 insertions(+), 1 deletions(-)

-- 
Generated by git-murpp 0.5.0


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH proxmox v6 1/2] router: cli: add confirmation helper

[Gabriel Goller]

Add confirmation helper that outputs a prompt and lets the user
confirm or deny it.
Implemented to close #4763.

Co-authored-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
---
 proxmox-router/Cargo.toml     |   1 +
 proxmox-router/src/cli/mod.rs | 113 +++++++++++++++++++++++++++++++++-
 2 files changed, 113 insertions(+), 1 deletion(-)

diff --git a/proxmox-router/Cargo.toml b/proxmox-router/Cargo.toml
index dcd71a4..0b9d361 100644
--- a/proxmox-router/Cargo.toml
+++ b/proxmox-router/Cargo.toml
@@ -19,6 +19,7 @@ percent-encoding.workspace = true
 serde_json.workspace = true
 serde.workspace = true
 unicode-width ="0.1.8"
+regex.workspace = true
 
 # cli:
 tokio = { workspace = true, features = [], optional = true }
diff --git a/proxmox-router/src/cli/mod.rs b/proxmox-router/src/cli/mod.rs
index 7df94ad..d13caa5 100644
--- a/proxmox-router/src/cli/mod.rs
+++ b/proxmox-router/src/cli/mod.rs
@@ -12,7 +12,11 @@
 //! - Ability to create interactive commands (using ``rustyline``)
 //! - Supports complex/nested commands
 
-use std::collections::HashMap;
+use std::{
+    collections::HashMap,
+    io::{self, Write},
+};
+use anyhow::bail;
 
 use crate::ApiMethod;
 
@@ -61,6 +65,113 @@ pub fn init_cli_logger(env_var_name: &str, default_log_level: &str) {
     .init();
 }
 
+#[derive(PartialEq, Debug)]
+/// Use for simple yes or no questions, where booleans can be confusing, especially if there's a
+/// default response to consider. The implementation provides query helper for the CLI.
+pub enum Confirmation {
+    Yes,
+    No,
+}
+
+impl Confirmation {
+    /// Get the formatted choice for the query prompt, with self being the highlighted (default)
+    /// one displayed as upper case.
+    pub fn default_choice_str(&self) -> &'static str {
+        match self {
+            Self::Yes => "Y/n",
+            Self::No => "y/N",
+        }
+    }
+
+    /// Returns true if the answer is Yes
+    pub fn is_yes(&self) -> bool {
+        *self == Self::Yes
+    }
+
+    /// Returns true if the answer is No
+    pub fn is_no(&self) -> bool {
+        *self == Self::No
+    }
+
+    /// Parse an input string reference as yes or no confirmation.
+    ///
+    /// The input string is checked verbatim if it is exactly one of the single chars 'y', 'Y',
+    /// 'n', or 'N'. You must trim the string before calling, if needed, or use one of the query
+    /// helper functions.
+    ///
+    /// ```
+    /// use proxmox_router::cli::Confirmation;
+    ///
+    /// let answer = Confirmation::from_str("y");
+    /// assert!(answer.expect("valid").is_yes());
+    ///
+    /// let answer = Confirmation::from_str("N");
+    /// assert!(answer.expect("valid").is_no());
+    ///
+    /// let answer = Confirmation::from_str("bogus");
+    /// assert!(answer.is_err());
+    /// ```
+    pub fn from_str(input: &str) -> Result<Self, anyhow::Error> {
+        match input.trim() {
+            "y" | "Y" => Ok(Self::Yes),
+            "n" | "N" => Ok(Self::No),
+            _ => bail!("unexpected choice '{input}'! Use 'y' or 'n'"),
+        }
+    }
+
+    /// Parse a input string reference as yes or no confirmation, allowing a fallback default
+    /// answer if the user enters an empty choice.
+    ///
+    /// The input string is checked verbatim if it is exactly one of the single chars 'y', 'Y',
+    /// 'n', or 'N'. The empty string maps to the default. You must trim the string before calling,
+    /// if needed, or use one of the query helper functions.
+    ///
+    /// ```
+    /// use proxmox_router::cli::Confirmation;
+    ///
+    /// let answer = Confirmation::from_str_with_default("", Confirmation::No);
+    /// assert!(answer.expect("valid").is_no());
+    ///
+    /// let answer = Confirmation::from_str_with_default("n", Confirmation::Yes);
+    /// assert!(answer.expect("valid").is_no());
+    ///
+    /// let answer = Confirmation::from_str_with_default("yes", Confirmation::Yes);
+    /// assert!(answer.is_err()); // full-word answer not allowed for now.
+    /// ```
+    pub fn from_str_with_default(input: &str, default: Self) -> Result<Self, anyhow::Error> {
+        match input.trim() {
+            "y" | "Y" => Ok(Self::Yes),
+            "n" | "N" => Ok(Self::No),
+            "" => Ok(default),
+            _ => bail!("unexpected choice '{input}'! Use enter for default or use 'y' or 'n'"),
+        }
+    }
+
+    /// Print a query prompt with available yes no choices and returns the String the user enters.
+    fn read_line(query: &str, choices: &str) -> Result<String, io::Error> {
+        print!("{query} [{choices}]: ");
+
+        io::stdout().flush()?;
+        let stdin = io::stdin();
+        let mut line = String::new();
+        stdin.read_line(&mut line)?;
+        Ok(line)
+    }
+
+    /// Print a query prompt and parse the white-space trimmed answer using from_str.
+    pub fn query(query: &str) -> Result<Self, anyhow::Error> {
+        let line = Self::read_line(query, "y/n")?;
+        Confirmation::from_str(line.trim())
+    }
+
+    /// Print a query prompt and parse the answer using from_str_with_default, falling back to the
+    /// default_answer if the user provided an empty string.
+    pub fn query_with_default(query: &str, default_answer: Self) -> Result<Self, anyhow::Error> {
+        let line = Self::read_line(query, default_answer.default_choice_str())?;
+        Confirmation::from_str_with_default(line.trim(), default_answer)
+    }
+}
+
 /// Define a simple CLI command.
 pub struct CliCommand {
     /// The Schema definition.
-- 
2.43.0



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH proxmox v6 1/2] router: cli: add confirmation helper

[Wolfgang Bumiller]

Some minor cleanups & doc nits:

On Fri, Apr 26, 2024 at 02:37:15PM GMT, Gabriel Goller wrote:
> Add confirmation helper that outputs a prompt and lets the user
> confirm or deny it.
> Implemented to close #4763.
> 
> Co-authored-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
> Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
> ---
>  proxmox-router/Cargo.toml     |   1 +
>  proxmox-router/src/cli/mod.rs | 113 +++++++++++++++++++++++++++++++++-
>  2 files changed, 113 insertions(+), 1 deletion(-)
> 
> diff --git a/proxmox-router/Cargo.toml b/proxmox-router/Cargo.toml
> index dcd71a4..0b9d361 100644
> --- a/proxmox-router/Cargo.toml
> +++ b/proxmox-router/Cargo.toml
> @@ -19,6 +19,7 @@ percent-encoding.workspace = true
>  serde_json.workspace = true
>  serde.workspace = true
>  unicode-width ="0.1.8"
> +regex.workspace = true
>  
>  # cli:
>  tokio = { workspace = true, features = [], optional = true }
> diff --git a/proxmox-router/src/cli/mod.rs b/proxmox-router/src/cli/mod.rs
> index 7df94ad..d13caa5 100644
> --- a/proxmox-router/src/cli/mod.rs
> +++ b/proxmox-router/src/cli/mod.rs
> @@ -12,7 +12,11 @@
>  //! - Ability to create interactive commands (using ``rustyline``)
>  //! - Supports complex/nested commands
>  
> -use std::collections::HashMap;
> +use std::{
> +    collections::HashMap,
> +    io::{self, Write},
> +};
> +use anyhow::bail;
>  
>  use crate::ApiMethod;
>  
> @@ -61,6 +65,113 @@ pub fn init_cli_logger(env_var_name: &str, default_log_level: &str) {
>      .init();
>  }
>  
> +#[derive(PartialEq, Debug)]

This is essentially a bool, so +Clone+Copy please. Also maybe +Eq?

> +/// Use for simple yes or no questions, where booleans can be confusing, especially if there's a
> +/// default response to consider. The implementation provides query helper for the CLI.
> +pub enum Confirmation {
> +    Yes,
> +    No,
> +}
> +
> +impl Confirmation {
> +    /// Get the formatted choice for the query prompt, with self being the highlighted (default)
> +    /// one displayed as upper case.
> +    pub fn default_choice_str(&self) -> &'static str {

^ With +Copy all the methods can drop the `&` and be self-consuming.

> +        match self {
> +            Self::Yes => "Y/n",
> +            Self::No => "y/N",
> +        }
> +    }
> +
> +    /// Returns true if the answer is Yes
> +    pub fn is_yes(&self) -> bool {
> +        *self == Self::Yes
> +    }
> +
> +    /// Returns true if the answer is No
> +    pub fn is_no(&self) -> bool {
> +        *self == Self::No
> +    }
> +
> +    /// Parse an input string reference as yes or no confirmation.
> +    ///
> +    /// The input string is checked verbatim if it is exactly one of the single chars 'y', 'Y',
> +    /// 'n', or 'N'. You must trim the string before calling, if needed, or use one of the query
> +    /// helper functions.
> +    ///
> +    /// ```
> +    /// use proxmox_router::cli::Confirmation;
> +    ///
> +    /// let answer = Confirmation::from_str("y");
> +    /// assert!(answer.expect("valid").is_yes());
> +    ///
> +    /// let answer = Confirmation::from_str("N");
> +    /// assert!(answer.expect("valid").is_no());
> +    ///
> +    /// let answer = Confirmation::from_str("bogus");
> +    /// assert!(answer.is_err());
> +    /// ```
> +    pub fn from_str(input: &str) -> Result<Self, anyhow::Error> {
> +        match input.trim() {
> +            "y" | "Y" => Ok(Self::Yes),
> +            "n" | "N" => Ok(Self::No),
> +            _ => bail!("unexpected choice '{input}'! Use 'y' or 'n'"),
> +        }
> +    }
> +
> +    /// Parse a input string reference as yes or no confirmation, allowing a fallback default
> +    /// answer if the user enters an empty choice.
> +    ///
> +    /// The input string is checked verbatim if it is exactly one of the single chars 'y', 'Y',
> +    /// 'n', or 'N'. The empty string maps to the default. You must trim the string before calling,
> +    /// if needed, or use one of the query helper functions.
> +    ///
> +    /// ```
> +    /// use proxmox_router::cli::Confirmation;
> +    ///
> +    /// let answer = Confirmation::from_str_with_default("", Confirmation::No);
> +    /// assert!(answer.expect("valid").is_no());
> +    ///
> +    /// let answer = Confirmation::from_str_with_default("n", Confirmation::Yes);
> +    /// assert!(answer.expect("valid").is_no());
> +    ///
> +    /// let answer = Confirmation::from_str_with_default("yes", Confirmation::Yes);
> +    /// assert!(answer.is_err()); // full-word answer not allowed for now.
> +    /// ```
> +    pub fn from_str_with_default(input: &str, default: Self) -> Result<Self, anyhow::Error> {
> +        match input.trim() {
> +            "y" | "Y" => Ok(Self::Yes),
> +            "n" | "N" => Ok(Self::No),
> +            "" => Ok(default),
> +            _ => bail!("unexpected choice '{input}'! Use enter for default or use 'y' or 'n'"),
> +        }
> +    }
> +
> +    /// Print a query prompt with available yes no choices and returns the String the user enters.
> +    fn read_line(query: &str, choices: &str) -> Result<String, io::Error> {
> +        print!("{query} [{choices}]: ");
> +
> +        io::stdout().flush()?;
> +        let stdin = io::stdin();
> +        let mut line = String::new();
> +        stdin.read_line(&mut line)?;
> +        Ok(line)
> +    }
> +
> +    /// Print a query prompt and parse the white-space trimmed answer using from_str.

Please add backticks around `from_str`.

> +    pub fn query(query: &str) -> Result<Self, anyhow::Error> {
> +        let line = Self::read_line(query, "y/n")?;
> +        Confirmation::from_str(line.trim())
> +    }
> +
> +    /// Print a query prompt and parse the answer using from_str_with_default, falling back to the

Please add backticks around `from_str_with_default`.

> +    /// default_answer if the user provided an empty string.
> +    pub fn query_with_default(query: &str, default_answer: Self) -> Result<Self, anyhow::Error> {
> +        let line = Self::read_line(query, default_answer.default_choice_str())?;
> +        Confirmation::from_str_with_default(line.trim(), default_answer)
> +    }
> +}


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH proxmox v6 1/2] router: cli: add confirmation helper

[Wolfgang Bumiller]

Missed something in the first response:
We should `use anyhow::Error` rather than using `Result<_, anyhow::Error>`
everywhere.

On Fri, Apr 26, 2024 at 02:37:15PM GMT, Gabriel Goller wrote:
> Add confirmation helper that outputs a prompt and lets the user
> confirm or deny it.
> Implemented to close #4763.
> 
> Co-authored-by: Thomas Lamprecht <t.lamprecht@proxmox.com>
> Signed-off-by: Gabriel Goller <g.goller@proxmox.com>
> ---
>  proxmox-router/Cargo.toml     |   1 +
>  proxmox-router/src/cli/mod.rs | 113 +++++++++++++++++++++++++++++++++-
>  2 files changed, 113 insertions(+), 1 deletion(-)
> 
> diff --git a/proxmox-router/Cargo.toml b/proxmox-router/Cargo.toml
> index dcd71a4..0b9d361 100644
> --- a/proxmox-router/Cargo.toml
> +++ b/proxmox-router/Cargo.toml
> @@ -19,6 +19,7 @@ percent-encoding.workspace = true
>  serde_json.workspace = true
>  serde.workspace = true
>  unicode-width ="0.1.8"
> +regex.workspace = true
>  
>  # cli:
>  tokio = { workspace = true, features = [], optional = true }
> diff --git a/proxmox-router/src/cli/mod.rs b/proxmox-router/src/cli/mod.rs
> index 7df94ad..d13caa5 100644
> --- a/proxmox-router/src/cli/mod.rs
> +++ b/proxmox-router/src/cli/mod.rs
> @@ -12,7 +12,11 @@
>  //! - Ability to create interactive commands (using ``rustyline``)
>  //! - Supports complex/nested commands
>  
> -use std::collections::HashMap;
> +use std::{
> +    collections::HashMap,
> +    io::{self, Write},
> +};

(And add an empty line in between the std and external `use` group
please.)

> +use anyhow::bail;
>  
>  use crate::ApiMethod;
>  


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

[pbs-devel] [PATCH proxmox-backup v6 2/2] close #4763: client: add command to forget backup group

[Gabriel Goller]

Add the command `proxmox-backup-client group forget <group>` 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 <g.goller@proxmox.com>
---
 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<Value, Error> {
+    let backup_group: BackupGroup = group.parse()?;
+    let repo = extract_repository_from_value(&param)?;
+    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)
+}
diff --git a/proxmox-backup-client/src/main.rs b/proxmox-backup-client/src/main.rs
index 9dbd3cd1..63669c59 100644
--- a/proxmox-backup-client/src/main.rs
+++ b/proxmox-backup-client/src/main.rs
@@ -72,6 +72,8 @@ mod catalog;
 pub use catalog::*;
 mod snapshot;
 pub use snapshot::*;
+mod group;
+pub use group::*;
 pub mod key;
 pub mod namespace;
 
@@ -1792,6 +1794,7 @@ fn main() {
         .insert("benchmark", benchmark_cmd_def)
         .insert("change-owner", change_owner_cmd_def)
         .insert("namespace", namespace::cli_map())
+        .insert("group", group_mgmt_cli())
         .alias(&["files"], &["snapshot", "files"])
         .alias(&["forget"], &["snapshot", "forget"])
         .alias(&["upload-log"], &["snapshot", "upload-log"])
-- 
2.43.0



_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel

Re: [pbs-devel] [PATCH proxmox-backup v6 2/2] close #4763: client: add command to forget backup group

[Wolfgang Bumiller]

style nit

On Fri, Apr 26, 2024 at 02:37:16PM GMT, Gabriel Goller wrote:
> Add the command `proxmox-backup-client group forget <group>` 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 <g.goller@proxmox.com>
> ---
>  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<Value, Error> {

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(&param)?;
> +    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

Re: [pbs-devel] [PATCH proxmox-backup v6 2/2] close #4763: client: add command to forget backup group

[Gabriel Goller]

Agree with everything, have submitted a v7 already!


_______________________________________________
pbs-devel mailing list
pbs-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel