all lists on lists.proxmox.com
 help / color / mirror / Atom feed
* [pbs-devel] [PATCH proxmox-backup v3] fix #4971: client: Improve output on successful snapshot deletion
@ 2023-10-16 11:40 Philipp Hufnagl
  2023-10-18  9:06 ` [pbs-devel] applied: " Thomas Lamprecht
  0 siblings, 1 reply; 2+ messages in thread
From: Philipp Hufnagl @ 2023-10-16 11:40 UTC (permalink / raw)
  To: pbs-devel

When a snapshot gets deleted (forgotten), the proxmox backup client
currently returns returns
"Result: {
  "data": null
}"

This feedback may confuse users therefore this patch removes the output.

Signed-off-by: Philipp Hufnagl <p.hufnagl@proxmox.com>
---

Changes since v1
* Remove output message

Changes since v2
* Change commit message

 proxmox-backup-client/src/snapshot.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/proxmox-backup-client/src/snapshot.rs b/proxmox-backup-client/src/snapshot.rs
index 4ca8015f..fcfe5840 100644
--- a/proxmox-backup-client/src/snapshot.rs
+++ b/proxmox-backup-client/src/snapshot.rs
@@ -188,13 +188,13 @@ async fn forget_snapshots(param: Value) -> Result<Value, Error> {
 
     let path = format!("api2/json/admin/datastore/{}/snapshots", repo.store());
 
-    let result = client
+    client
         .delete(&path, Some(snapshot_args(&backup_ns, &snapshot)?))
         .await?;
 
     record_repository(&repo);
 
-    Ok(result)
+    Ok(Value::Null)
 }
 
 #[api(
-- 
2.39.2





^ permalink raw reply	[flat|nested] 2+ messages in thread

* [pbs-devel] applied: [PATCH proxmox-backup v3] fix #4971: client: Improve output on successful snapshot deletion
  2023-10-16 11:40 [pbs-devel] [PATCH proxmox-backup v3] fix #4971: client: Improve output on successful snapshot deletion Philipp Hufnagl
@ 2023-10-18  9:06 ` Thomas Lamprecht
  0 siblings, 0 replies; 2+ messages in thread
From: Thomas Lamprecht @ 2023-10-18  9:06 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion, Philipp Hufnagl

Am 16/10/2023 um 13:40 schrieb Philipp Hufnagl:
> When a snapshot gets deleted (forgotten), the proxmox backup client
> currently returns returns
> "Result: {
>   "data": null
> }"
> 
> This feedback may confuse users therefore this patch removes the output.
> 
> Signed-off-by: Philipp Hufnagl <p.hufnagl@proxmox.com>
> ---
> 
> Changes since v1
> * Remove output message
> 
> Changes since v2
> * Change commit message
> 
>  proxmox-backup-client/src/snapshot.rs | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)


applied, with a small follow-up (see below), thanks!

> 
> diff --git a/proxmox-backup-client/src/snapshot.rs b/proxmox-backup-client/src/snapshot.rs
> index 4ca8015f..fcfe5840 100644
> --- a/proxmox-backup-client/src/snapshot.rs
> +++ b/proxmox-backup-client/src/snapshot.rs
> @@ -188,13 +188,13 @@ async fn forget_snapshots(param: Value) -> Result<Value, Error> {
>  
>      let path = format!("api2/json/admin/datastore/{}/snapshots", repo.store());
>  
> -    let result = client
> +    client
>          .delete(&path, Some(snapshot_args(&backup_ns, &snapshot)?))
>          .await?;
>  
>      record_repository(&repo);
>  
> -    Ok(result)
> +    Ok(Value::Null)

One small thing, sorry to not have caught this earlier, but as we no
never return a serde_json::Value, it'd a bit cleaner to return an empty
tuple here via `Ok(())` and change the functions' signature
respectively.

I fixed that in a follow-up, IMO it was to small of a change to warrant
a v4 here.




^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2023-10-18  9:07 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-16 11:40 [pbs-devel] [PATCH proxmox-backup v3] fix #4971: client: Improve output on successful snapshot deletion Philipp Hufnagl
2023-10-18  9:06 ` [pbs-devel] applied: " Thomas Lamprecht

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal