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 18B9E69E4C; Mon, 16 Nov 2020 10:42:54 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with UTF8SMTP id 0C1302C9BD; Mon, 16 Nov 2020 10:42:54 +0100 (CET) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [212.186.127.180]) (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 firstgate.proxmox.com (Proxmox) with UTF8SMTPS id 55ABB2C9B0; Mon, 16 Nov 2020 10:42:53 +0100 (CET) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with UTF8SMTP id 0E0EE4357D; Mon, 16 Nov 2020 10:42:53 +0100 (CET) To: Thomas Lamprecht , Proxmox VE development discussion , pbs-devel@lists.proxmox.com References: <20201113093852.28788-1-d.csapak@proxmox.com> <20201113093852.28788-3-d.csapak@proxmox.com> <49f2d016-5674-e20d-6dc4-475fdd2b7fb7@proxmox.com> From: Dominik Csapak Message-ID: Date: Mon, 16 Nov 2020 10:42:51 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:83.0) Gecko/20100101 Thunderbird/83.0 MIME-Version: 1.0 In-Reply-To: <49f2d016-5674-e20d-6dc4-475fdd2b7fb7@proxmox.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-SPAM-LEVEL: Spam detection results: 0 AWL 0.358 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.001 Looks like a legit reply (A) RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust 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. [mod.rs, proxmox-backup-client.rs, snapshot.rs] Subject: Re: [pve-devel] [PATCH proxmox-backup 2/2] client: add 'snapshot notes show/update' command X-BeenThere: pve-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox VE development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 16 Nov 2020 09:42:54 -0000 On 11/16/20 10:37 AM, Thomas Lamprecht wrote: > On 13.11.20 10:38, Dominik Csapak wrote: >> to show and update snapshot notes from the cli >> >> Signed-off-by: Dominik Csapak >> --- >> src/bin/proxmox-backup-client.rs | 1 + >> src/bin/proxmox_backup_client/mod.rs | 2 + >> src/bin/proxmox_backup_client/snapshot.rs | 126 ++++++++++++++++++++++ >> 3 files changed, 129 insertions(+) >> create mode 100644 src/bin/proxmox_backup_client/snapshot.rs >> >> diff --git a/src/bin/proxmox-backup-client.rs b/src/bin/proxmox-backup-client.rs >> index 54e11f08..1581fbd2 100644 >> --- a/src/bin/proxmox-backup-client.rs >> +++ b/src/bin/proxmox-backup-client.rs >> @@ -2068,6 +2068,7 @@ fn main() { >> .insert("prune", prune_cmd_def) >> .insert("restore", restore_cmd_def) >> .insert("snapshots", snapshots_cmd_def) >> + .insert("snapshot", snapshot_mgtm_cli()) > > makes the CLI interface slightly weirder, maybe moving `snapshots` > into `snapshot list` could help (with hidden alias for backward compat)? mhmm.. yeah, but we do not have an alias for now AFAIK i just did not want to add two new top-level commands > > >> .insert("files", files_cmd_def) >> .insert("status", status_cmd_def) >> .insert("key", key::cli()) >> diff --git a/src/bin/proxmox_backup_client/mod.rs b/src/bin/proxmox_backup_client/mod.rs >> index 0c4bffb9..a14b0dc1 100644 >> --- a/src/bin/proxmox_backup_client/mod.rs >> +++ b/src/bin/proxmox_backup_client/mod.rs >> @@ -8,6 +8,8 @@ mod task; >> pub use task::*; >> mod catalog; >> pub use catalog::*; >> +mod snapshot; >> +pub use snapshot::*; >> >> pub mod key; >> >> diff --git a/src/bin/proxmox_backup_client/snapshot.rs b/src/bin/proxmox_backup_client/snapshot.rs >> new file mode 100644 >> index 00000000..ee794275 >> --- /dev/null >> +++ b/src/bin/proxmox_backup_client/snapshot.rs >> @@ -0,0 +1,126 @@ >> +use anyhow::Error; >> +use serde_json::{json, Value}; >> + >> +use proxmox::api::{api, cli::*}; >> +use proxmox_backup::tools; >> + >> +use crate::{ >> + complete_backup_snapshot, connect, extract_repository_from_value, BackupDir, REPO_URL_SCHEMA, >> +}; >> + >> +#[api( >> + input: { >> + properties: { >> + repository: { >> + schema: REPO_URL_SCHEMA, >> + optional: true, >> + }, >> + snapshot: { >> + type: String, >> + description: "Snapshot path.", >> + }, >> + "output-format": { >> + schema: OUTPUT_FORMAT, >> + optional: true, >> + }, >> + } >> + } >> +)] >> +/// Show notes >> +async fn show_notes(param: Value) -> Result { >> + let repo = extract_repository_from_value(¶m)?; >> + let path = tools::required_string_param(¶m, "snapshot")?; >> + >> + let snapshot: BackupDir = path.parse()?; >> + let client = connect(&repo)?; >> + >> + let path = format!("api2/json/admin/datastore/{}/notes", repo.store()); >> + >> + let args = json!({ >> + "backup-type": snapshot.group().backup_type(), >> + "backup-id": snapshot.group().backup_id(), >> + "backup-time": snapshot.backup_time(), >> + }); >> + >> + let output_format = get_output_format(¶m); > > slightly weird to have this here, in between the args value used below, as it's > only used after that. ok > >> + >> + let mut result = client.get(&path, Some(args)).await?; >> + >> + let comment = result["data"].take(); >> + >> + if output_format == "text" { >> + if let Some(comment) = comment.as_str() { >> + println!("{}", comment); >> + } >> + } else { >> + format_and_print_result( >> + &json!({ >> + "comment": comment, > > We could just return the notes directly, without wrapping into an object? no, because the proxmox-backup-client calls in pve-storage always use --output-format json and tries to decode that (and fails in case of a single string without options) the default call just prints it anyway and having "proper" json makes it easier to consume for others (it depends on the parser if a standalone string is valid json) > >> + }), >> + &output_format, >> + ); >> + } >> + >> + Ok(Value::Null) >> +} >> + >> +#[api( >> + input: { >> + properties: { >> + repository: { >> + schema: REPO_URL_SCHEMA, >> + optional: true, >> + }, >> + snapshot: { >> + type: String, >> + description: "Snapshot path.", >> + }, >> + notes: { >> + type: String, >> + description: "The Notes.", >> + }, >> + } >> + } >> +)] >> +/// Update Notes >> +async fn update_notes(param: Value) -> Result { >> + let repo = extract_repository_from_value(¶m)?; >> + let path = tools::required_string_param(¶m, "snapshot")?; >> + let notes = tools::required_string_param(¶m, "notes")?; >> + >> + let snapshot: BackupDir = path.parse()?; >> + let mut client = connect(&repo)?; >> + >> + let path = format!("api2/json/admin/datastore/{}/notes", repo.store()); >> + >> + let args = json!({ >> + "backup-type": snapshot.group().backup_type(), >> + "backup-id": snapshot.group().backup_id(), >> + "backup-time": snapshot.backup_time(), >> + "notes": notes, >> + }); >> + >> + client.put(&path, Some(args)).await?; >> + >> + Ok(Value::Null) >> +} >> + >> +fn notes_cli() -> CliCommandMap { >> + CliCommandMap::new() >> + .insert( >> + "show", >> + CliCommand::new(&API_METHOD_SHOW_NOTES) >> + .arg_param(&["snapshot"]) >> + .completion_cb("snapshot", complete_backup_snapshot), >> + ) >> + .insert( >> + "update", >> + CliCommand::new(&API_METHOD_UPDATE_NOTES) >> + .arg_param(&["snapshot", "notes"]) >> + .completion_cb("snapshot", complete_backup_snapshot), >> + ) >> +} >> + >> +pub fn snapshot_mgtm_cli() -> CliCommandMap { >> + CliCommandMap::new().insert("notes", notes_cli()) >> +} >> > >