public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
From: Christian Ebner <c.ebner@proxmox.com>
To: pbs-devel@lists.proxmox.com
Subject: [pbs-devel] [PATCH proxmox-backup 2/4] server: push: consistently use remote over target for error messages
Date: Thu, 21 Nov 2024 16:43:35 +0100	[thread overview]
Message-ID: <20241121154337.471425-3-c.ebner@proxmox.com> (raw)
In-Reply-To: <20241121154337.471425-1-c.ebner@proxmox.com>

Mixing of terms only makes the errors harder to understand.

In order to make error messages more intuitive, always refer to the
sync push target as remote, mention the remote explicitly and/or
improve messages where needed.

Signed-off-by: Christian Ebner <c.ebner@proxmox.com>
---
 src/server/push.rs | 35 +++++++++++++++++++++--------------
 1 file changed, 21 insertions(+), 14 deletions(-)

diff --git a/src/server/push.rs b/src/server/push.rs
index 2181634c6..86cef5520 100644
--- a/src/server/push.rs
+++ b/src/server/push.rs
@@ -197,7 +197,7 @@ async fn remove_target_namespace(
     target_namespace: &BackupNamespace,
 ) -> Result<BackupGroupDeleteStats, Error> {
     if target_namespace.is_root() {
-        bail!("cannot remove root namespace from target");
+        bail!("Cannot remove root namespace from remote");
     }
 
     check_ns_remote_datastore_privs(params, target_namespace, PRIV_REMOTE_DATASTORE_MODIFY)
@@ -295,7 +295,7 @@ async fn check_or_create_target_namespace(
         // Sub-namespaces have to be created by creating parent components first.
 
         check_ns_remote_datastore_privs(params, target_namespace, PRIV_REMOTE_DATASTORE_MODIFY)
-            .map_err(|err| format_err!("Creating namespace not allowed - {err}"))?;
+            .map_err(|err| format_err!("Creating remote namespace not allowed - {err}"))?;
 
         let mut parent = BackupNamespace::root();
         for component in target_namespace.components() {
@@ -311,7 +311,7 @@ async fn check_or_create_target_namespace(
                 args["parent"] = serde_json::to_value(parent.clone())?;
             }
             match params.target.client.post(&api_path, Some(args)).await {
-                Ok(_) => info!("Created new namespace on target: {current}"),
+                Ok(_) => info!("Successfully created new namespace {current} on remote"),
                 Err(err) => {
                     bail!("Remote creation of namespace {current} failed, remote returned: {err}")
                 }
@@ -445,18 +445,19 @@ pub(crate) async fn push_store(mut params: PushParameters) -> Result<SyncStats,
                     }));
                     if delete_stats.protected_snapshots() > 0 {
                         warn!(
-                            "kept {protected_count} protected snapshots of namespace '{target_namespace}'",
+                            "Kept {protected_count} protected snapshots of remote namespace {target_namespace}",
                             protected_count = delete_stats.protected_snapshots(),
                         );
                         continue;
                     }
                 }
                 Err(err) => {
-                    warn!("failed to remove vanished namespace {target_namespace} - {err}");
+                    warn!("Encountered errors: {err}");
+                    warn!("Failed to remove vanished namespace {target_namespace} from remote!");
                     continue;
                 }
             }
-            info!("removed vanished namespace {target_namespace}");
+            info!("Successfully removed vanished namespace {target_namespace} from remote");
         }
 
         if !params.target.supports_prune_delete_stats {
@@ -481,7 +482,7 @@ pub(crate) async fn push_namespace(
     let target_namespace = params.map_to_target(namespace)?;
     // Check if user is allowed to perform backups on remote datastore
     check_ns_remote_datastore_privs(params, &target_namespace, PRIV_REMOTE_DATASTORE_BACKUP)
-        .map_err(|err| format_err!("Pushing to remote not allowed - {err}"))?;
+        .map_err(|err| format_err!("Pushing to remote namespace not allowed - {err}"))?;
 
     let mut list: Vec<BackupGroup> = params
         .source
@@ -527,7 +528,8 @@ pub(crate) async fn push_namespace(
         match push_group(params, namespace, &group, &mut progress).await {
             Ok(sync_stats) => stats.add(sync_stats),
             Err(err) => {
-                warn!("sync group '{group}' failed  - {err}");
+                warn!("Encountered errors: {err}");
+                warn!("Failed to push group {group} to remote!");
                 errors = true;
             }
         }
@@ -543,13 +545,13 @@ pub(crate) async fn push_namespace(
                 continue;
             }
 
-            info!("delete vanished group '{target_group}'");
+            info!("Removed vanished group {target_group} from remote");
 
             match remove_target_group(params, &target_namespace, &target_group).await {
                 Ok(delete_stats) => {
                     if delete_stats.protected_snapshots() > 0 {
                         warn!(
-                            "kept {protected_count} protected snapshots of group '{target_group}'",
+                            "Kept {protected_count} protected snapshots of group {target_group} on remote",
                             protected_count = delete_stats.protected_snapshots(),
                         );
                     }
@@ -560,7 +562,8 @@ pub(crate) async fn push_namespace(
                     }));
                 }
                 Err(err) => {
-                    warn!("failed to delete vanished group - {err}");
+                    warn!("Encountered errors: {err}");
+                    warn!("Failed to remove vanished group {target_group} from remote!");
                     errors = true;
                     continue;
                 }
@@ -693,7 +696,7 @@ pub(crate) async fn push_group(
             }
             if snapshot.protected {
                 info!(
-                    "don't delete vanished snapshot {name} (protected)",
+                    "Kept protected snapshot {name} on remote",
                     name = snapshot.backup
                 );
                 continue;
@@ -701,12 +704,16 @@ pub(crate) async fn push_group(
             if let Err(err) =
                 forget_target_snapshot(params, &target_namespace, &snapshot.backup).await
             {
+                info!("Encountered errors: {err}");
                 info!(
-                    "could not delete vanished snapshot {name} - {err}",
+                    "Failed to remove vanished snapshot {name} from remote!",
                     name = snapshot.backup
                 );
             }
-            info!("delete vanished snapshot {name}", name = snapshot.backup);
+            info!(
+                "Removed vanished snapshot {name} from remote",
+                name = snapshot.backup
+            );
             stats.add(SyncStats::from(RemovedVanishedStats {
                 snapshots: 1,
                 groups: 0,
-- 
2.39.5



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


  parent reply	other threads:[~2024-11-21 15:43 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-11-21 15:43 [pbs-devel] [PATCH proxmox-backup 0/4] improve push sync job log messages Christian Ebner
2024-11-21 15:43 ` [pbs-devel] [PATCH proxmox-backup 1/4] server: push: fix needless borrow clippy warning Christian Ebner
2024-11-21 15:43 ` Christian Ebner [this message]
2024-11-21 15:43 ` [pbs-devel] [PATCH proxmox-backup 3/4] server: push: add error context to all target api calls Christian Ebner
2024-11-22  9:01   ` Fabian Grünbichler
2024-11-22 10:11     ` Christian Ebner
2024-11-21 15:43 ` [pbs-devel] [PATCH proxmox-backup 4/4] server: push: various smaller improvements to error messages Christian Ebner
2024-11-21 16:06 ` [pbs-devel] [PATCH proxmox-backup 0/4] improve push sync job log messages Gabriel Goller
2024-11-21 16:26   ` Christian Ebner
2024-11-21 17:04     ` Gabriel Goller
2024-11-21 19:32       ` Fabian Grünbichler
2024-11-22  8:41         ` Christian Ebner
2024-11-22 12:28   ` Christian Ebner
2024-11-22  9:04 ` [pbs-devel] partially-applied: " Fabian Grünbichler

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20241121154337.471425-3-c.ebner@proxmox.com \
    --to=c.ebner@proxmox.com \
    --cc=pbs-devel@lists.proxmox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Service provided by Proxmox Server Solutions GmbH | Privacy | Legal