Re: [pve-devel] [PATCH proxmox-backup 2/2] client: add 'snapshot notes show/update' command

[Thomas Lamprecht <t.lamprecht@proxmox.com>]

On 13.11.20 10:38, Dominik Csapak wrote:
> to show and update snapshot notes from the cli
> 
> Signed-off-by: Dominik Csapak <d.csapak@proxmox.com>
> ---
>  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)?


>          .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<Value, Error> {
> +    let repo = extract_repository_from_value(&param)?;
> +    let path = tools::required_string_param(&param, "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(&param);

slightly weird to have this here, in between the args value used below, as it's
only used after that.

> +
> +    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?

> +            }),
> +            &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<Value, Error> {
> +    let repo = extract_repository_from_value(&param)?;
> +    let path = tools::required_string_param(&param, "snapshot")?;
> +    let notes = tools::required_string_param(&param, "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())
> +}
>