* [pbs-devel] [PATCH proxmox] router: cli: added `ask_for_confirmation` helper @ 2023-08-30 12:45 Gabriel Goller 2023-08-30 12:45 ` [pbs-devel] [PATCH proxmox-backup v2] close #4763: client: added command to forget backup group Gabriel Goller 2023-09-07 15:26 ` [pbs-devel] [PATCH proxmox] router: cli: added `ask_for_confirmation` helper Thomas Lamprecht 0 siblings, 2 replies; 5+ messages in thread From: Gabriel Goller @ 2023-08-30 12:45 UTC (permalink / raw) To: pbs-devel Added `ask_for_confirmation` helper that outputs a prompt and lets the user confirm or deny it. Implemented to close #4763. Signed-off-by: Gabriel Goller <g.goller@proxmox.com> --- Note: A dependency bump is needed. proxmox-router/src/cli/mod.rs | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/proxmox-router/src/cli/mod.rs b/proxmox-router/src/cli/mod.rs index 208df4a..2366b47 100644 --- a/proxmox-router/src/cli/mod.rs +++ b/proxmox-router/src/cli/mod.rs @@ -12,11 +12,15 @@ //! - Ability to create interactive commands (using ``rustyline``) //! - Supports complex/nested commands -use std::collections::HashMap; +use std::{ + collections::HashMap, + io::{self, Write}, +}; use crate::ApiMethod; mod environment; +use anyhow::bail; pub use environment::*; mod shellword; @@ -62,6 +66,19 @@ pub fn init_cli_logger(env_var_name: &str, default_log_level: &str) { .init(); } +pub fn ask_for_confirmation(query: String) -> Result<(), anyhow::Error> { + print!("{} [y/N]: ", query); + io::stdout().flush()?; + let stdin = io::stdin(); + let mut line = String::new(); + stdin.read_line(&mut line)?; + if line.trim() == "y" { + Ok(()) + } else { + bail!("Aborted"); + } +} + /// Define a simple CLI command. pub struct CliCommand { /// The Schema definition. -- 2.39.2 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [pbs-devel] [PATCH proxmox-backup v2] close #4763: client: added command to forget backup group 2023-08-30 12:45 [pbs-devel] [PATCH proxmox] router: cli: added `ask_for_confirmation` helper Gabriel Goller @ 2023-08-30 12:45 ` Gabriel Goller 2023-09-07 15:26 ` [pbs-devel] [PATCH proxmox] router: cli: added `ask_for_confirmation` helper Thomas Lamprecht 1 sibling, 0 replies; 5+ messages in thread From: Gabriel Goller @ 2023-08-30 12:45 UTC (permalink / raw) To: pbs-devel Added 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. Fixed paths for local dependencies in Cargo.toml. Signed-off-by: Gabriel Goller <g.goller@proxmox.com> --- Note: The dependency to `proxmox-router` needs to be bumped to include the change in the other patch. proxmox-backup-client/src/group.rs | 66 ++++++++++++++++++++++++++++++ proxmox-backup-client/src/main.rs | 3 ++ 2 files changed, 69 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..93b97a59 --- /dev/null +++ b/proxmox-backup-client/src/group.rs @@ -0,0 +1,66 @@ +use anyhow::Error; +use serde_json::Value; + +use proxmox_router::cli::{ask_for_confirmation, CliCommand, CliCommandMap}; +use proxmox_schema::api; + +use pbs_api_types::{BackupGroup, BackupNamespace}; +use pbs_client::tools::{connect, extract_repository_from_value}; +use crate::{ + complete_backup_group, complete_namespace, complete_repository, merge_group_into, + REPO_URL_SCHEMA, +}; + +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(¶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(); + + ask_for_confirmation(format!( + "Delete group \"{}\" with {} snapshot(s)?", + backup_group, snapshots + ))?; + + let path = format!("api2/json/admin/datastore/{}/groups", repo.store()); + let _ = client.delete(&path, Some(api_params)).await?; + + println!("Successfully deleted group!"); + Ok(Value::Null) +} diff --git a/proxmox-backup-client/src/main.rs b/proxmox-backup-client/src/main.rs index 1a13291a..975682de 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; @@ -1783,6 +1785,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.39.2 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [pbs-devel] [PATCH proxmox] router: cli: added `ask_for_confirmation` helper 2023-08-30 12:45 [pbs-devel] [PATCH proxmox] router: cli: added `ask_for_confirmation` helper Gabriel Goller 2023-08-30 12:45 ` [pbs-devel] [PATCH proxmox-backup v2] close #4763: client: added command to forget backup group Gabriel Goller @ 2023-09-07 15:26 ` Thomas Lamprecht 2023-09-08 10:39 ` Gabriel Goller 2023-09-08 11:44 ` Gabriel Goller 1 sibling, 2 replies; 5+ messages in thread From: Thomas Lamprecht @ 2023-09-07 15:26 UTC (permalink / raw) To: Proxmox Backup Server development discussion, Gabriel Goller Cc: Wolfgang Bumiller, Max Carrara On 30/08/2023 14:45, Gabriel Goller wrote: > Added `ask_for_confirmation` helper that outputs a prompt and > lets the user confirm or deny it. Implemented to close #4763. > > Signed-off-by: Gabriel Goller <g.goller@proxmox.com> > --- > > Note: A dependency bump is needed. > > proxmox-router/src/cli/mod.rs | 19 ++++++++++++++++++- > 1 file changed, 18 insertions(+), 1 deletion(-) > > diff --git a/proxmox-router/src/cli/mod.rs b/proxmox-router/src/cli/mod.rs > index 208df4a..2366b47 100644 > --- a/proxmox-router/src/cli/mod.rs > +++ b/proxmox-router/src/cli/mod.rs > @@ -12,11 +12,15 @@ > //! - Ability to create interactive commands (using ``rustyline``) > //! - Supports complex/nested commands > > -use std::collections::HashMap; > +use std::{ > + collections::HashMap, > + io::{self, Write}, > +}; > > use crate::ApiMethod; > > mod environment; > +use anyhow::bail; hmm, anyhow on library code is a slight smell, not sure about our verdict here, for API stuff it's widely used anyway – @Wolfgang (or @Max, you checked out error handling too here IIRC) > pub use environment::*; > > mod shellword; > @@ -62,6 +66,19 @@ pub fn init_cli_logger(env_var_name: &str, default_log_level: &str) { > .init(); > } > > +pub fn ask_for_confirmation(query: String) -> Result<(), anyhow::Error> { > + print!("{} [y/N]: ", query); here too w.r.t. format strings and params please: "{query} [y/N]" > + io::stdout().flush()?; > + let stdin = io::stdin(); > + let mut line = String::new(); > + stdin.read_line(&mut line)?; don't we have already some pty/tty helpers that we use for passwords that we could re-use here? Not a hard recommendation or the like, just interest. > + if line.trim() == "y" { Hmm, not locale-aware though. Albeit we do not place that much of an emphasis on being so in our CLI tools. Would be rather just interesting if there are sane wrappers or implementations for something like nl_langinfo's [0] YESEXPR or NOEXPR fom libc in rust. [0]: https://manpages.debian.org/bookworm/manpages-dev/nl_langinfo.3.en.html Anyhow, not blocking this, just stuck out – would at least compare the char case-insensitive though. > + Ok(()) > + } else { > + bail!("Aborted"); > + } > +} > + > /// Define a simple CLI command. > pub struct CliCommand { > /// The Schema definition. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [pbs-devel] [PATCH proxmox] router: cli: added `ask_for_confirmation` helper 2023-09-07 15:26 ` [pbs-devel] [PATCH proxmox] router: cli: added `ask_for_confirmation` helper Thomas Lamprecht @ 2023-09-08 10:39 ` Gabriel Goller 2023-09-08 11:44 ` Gabriel Goller 1 sibling, 0 replies; 5+ messages in thread From: Gabriel Goller @ 2023-09-08 10:39 UTC (permalink / raw) To: Thomas Lamprecht, Proxmox Backup Server development discussion Cc: Wolfgang Bumiller, Max Carrara On 9/7/23 17:26, Thomas Lamprecht wrote: > On 30/08/2023 14:45, Gabriel Goller wrote: >> Added `ask_for_confirmation` helper that outputs a prompt and >> lets the user confirm or deny it. Implemented to close #4763. >> >> Signed-off-by: Gabriel Goller <g.goller@proxmox.com> >> --- >> >> Note: A dependency bump is needed. >> >> proxmox-router/src/cli/mod.rs | 19 ++++++++++++++++++- >> 1 file changed, 18 insertions(+), 1 deletion(-) >> >> diff --git a/proxmox-router/src/cli/mod.rs b/proxmox-router/src/cli/mod.rs >> index 208df4a..2366b47 100644 >> --- a/proxmox-router/src/cli/mod.rs >> +++ b/proxmox-router/src/cli/mod.rs >> @@ -12,11 +12,15 @@ >> //! - Ability to create interactive commands (using ``rustyline``) >> //! - Supports complex/nested commands >> >> -use std::collections::HashMap; >> +use std::{ >> + collections::HashMap, >> + io::{self, Write}, >> +}; >> >> use crate::ApiMethod; >> >> mod environment; >> +use anyhow::bail; > hmm, anyhow on library code is a slight smell, not sure about our verdict > here, for API stuff it's widely used anyway – @Wolfgang (or @Max, you checked > out error handling too here IIRC) Discussed with Max, decided to use `io:Error`. >> pub use environment::*; >> >> mod shellword; >> @@ -62,6 +66,19 @@ pub fn init_cli_logger(env_var_name: &str, default_log_level: &str) { >> .init(); >> } >> >> +pub fn ask_for_confirmation(query: String) -> Result<(), anyhow::Error> { >> + print!("{} [y/N]: ", query); > here too w.r.t. format strings and params please: > > "{query} [y/N]" > >> + io::stdout().flush()?; >> + let stdin = io::stdin(); >> + let mut line = String::new(); >> + stdin.read_line(&mut line)?; > don't we have already some pty/tty helpers that we use for passwords that > we could re-use here? Not a hard recommendation or the like, just interest. We only have a helper for the tty output (`TtyOutput`), which is used to write the asterisks for the password input. I don't think this is useful in this case. >> + if line.trim() == "y" { > Hmm, not locale-aware though. Albeit we do not place that much of an > emphasis on being so in our CLI tools. > > Would be rather just interesting if there are sane wrappers or > implementations for something like nl_langinfo's [0] YESEXPR or NOEXPR > fom libc in rust. > > [0]: https://manpages.debian.org/bookworm/manpages-dev/nl_langinfo.3.en.html Yes, this is nice. I will use `YESEXPR` to match the input from the cli. To show a localized prompt (the [y/n] thing) I'll extract the lowercase character from the `YESEXPR` and `NOEXPR` (Which is a bit hacky, but sadly the `NOSTR` and `YESSTR` parameter are deprecated :( ). > Anyhow, not blocking this, just stuck out – would at least compare > the char case-insensitive though. >> + Ok(()) >> + } else { >> + bail!("Aborted"); >> + } >> +} >> + >> /// Define a simple CLI command. >> pub struct CliCommand { >> /// The Schema definition. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [pbs-devel] [PATCH proxmox] router: cli: added `ask_for_confirmation` helper 2023-09-07 15:26 ` [pbs-devel] [PATCH proxmox] router: cli: added `ask_for_confirmation` helper Thomas Lamprecht 2023-09-08 10:39 ` Gabriel Goller @ 2023-09-08 11:44 ` Gabriel Goller 1 sibling, 0 replies; 5+ messages in thread From: Gabriel Goller @ 2023-09-08 11:44 UTC (permalink / raw) To: Thomas Lamprecht, Proxmox Backup Server development discussion Cc: Wolfgang Bumiller, Max Carrara Submitted a new version of the series. On 9/7/23 17:26, Thomas Lamprecht wrote: > On 30/08/2023 14:45, Gabriel Goller wrote: >> Added `ask_for_confirmation` helper that outputs a prompt and >> lets the user confirm or deny it. Implemented to close #4763. >> >> Signed-off-by: Gabriel Goller <g.goller@proxmox.com> >> --- >> >> Note: A dependency bump is needed. >> >> proxmox-router/src/cli/mod.rs | 19 ++++++++++++++++++- >> 1 file changed, 18 insertions(+), 1 deletion(-) >> >> diff --git a/proxmox-router/src/cli/mod.rs b/proxmox-router/src/cli/mod.rs >> index 208df4a..2366b47 100644 >> --- a/proxmox-router/src/cli/mod.rs >> +++ b/proxmox-router/src/cli/mod.rs >> @@ -12,11 +12,15 @@ >> //! - Ability to create interactive commands (using ``rustyline``) >> //! - Supports complex/nested commands >> >> -use std::collections::HashMap; >> +use std::{ >> + collections::HashMap, >> + io::{self, Write}, >> +}; >> >> use crate::ApiMethod; >> >> mod environment; >> +use anyhow::bail; > hmm, anyhow on library code is a slight smell, not sure about our verdict > here, for API stuff it's widely used anyway – @Wolfgang (or @Max, you checked > out error handling too here IIRC) > >> pub use environment::*; >> >> mod shellword; >> @@ -62,6 +66,19 @@ pub fn init_cli_logger(env_var_name: &str, default_log_level: &str) { >> .init(); >> } >> >> +pub fn ask_for_confirmation(query: String) -> Result<(), anyhow::Error> { >> + print!("{} [y/N]: ", query); > here too w.r.t. format strings and params please: > > "{query} [y/N]" > >> + io::stdout().flush()?; >> + let stdin = io::stdin(); >> + let mut line = String::new(); >> + stdin.read_line(&mut line)?; > don't we have already some pty/tty helpers that we use for passwords that > we could re-use here? Not a hard recommendation or the like, just interest. > >> + if line.trim() == "y" { > Hmm, not locale-aware though. Albeit we do not place that much of an > emphasis on being so in our CLI tools. > > Would be rather just interesting if there are sane wrappers or > implementations for something like nl_langinfo's [0] YESEXPR or NOEXPR > fom libc in rust. > > [0]: https://manpages.debian.org/bookworm/manpages-dev/nl_langinfo.3.en.html > > Anyhow, not blocking this, just stuck out – would at least compare > the char case-insensitive though. > >> + Ok(()) >> + } else { >> + bail!("Aborted"); >> + } >> +} >> + >> /// Define a simple CLI command. >> pub struct CliCommand { >> /// The Schema definition. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-09-08 11:45 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-08-30 12:45 [pbs-devel] [PATCH proxmox] router: cli: added `ask_for_confirmation` helper Gabriel Goller 2023-08-30 12:45 ` [pbs-devel] [PATCH proxmox-backup v2] close #4763: client: added command to forget backup group Gabriel Goller 2023-09-07 15:26 ` [pbs-devel] [PATCH proxmox] router: cli: added `ask_for_confirmation` helper Thomas Lamprecht 2023-09-08 10:39 ` Gabriel Goller 2023-09-08 11:44 ` Gabriel Goller
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox