public inbox for pbs-devel@lists.proxmox.com
 help / color / mirror / Atom feed
* [pbs-devel] [PATCH pbs 1/2] pbs-client: backup-writer: use log::warn instead of eprintln!
@ 2023-06-07 14:07 Maximiliano Sandoval
  2023-06-07 14:07 ` [pbs-devel] [PATCH pbs 2/2] fix #4761: pbs-client: backup_writer: improve error reporting Maximiliano Sandoval
  2023-07-19  9:08 ` [pbs-devel] applied-series: [PATCH pbs 1/2] pbs-client: backup-writer: use log::warn instead of eprintln! Fabian Grünbichler
  0 siblings, 2 replies; 4+ messages in thread
From: Maximiliano Sandoval @ 2023-06-07 14:07 UTC (permalink / raw)
  To: pbs-devel

The errors are not fatal so we only throw a warning.

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
 pbs-client/src/backup_writer.rs | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/pbs-client/src/backup_writer.rs b/pbs-client/src/backup_writer.rs
index be6da2a6..2e18a060 100644
--- a/pbs-client/src/backup_writer.rs
+++ b/pbs-client/src/backup_writer.rs
@@ -293,7 +293,7 @@ impl BackupWriter {
                         )
                         .await
                     {
-                        eprintln!("Error downloading .fidx from previous manifest: {}", err);
+                        log::warn!("Error downloading .fidx from previous manifest: {}", err);
                     }
                 }
                 Ok(ArchiveType::DynamicIndex) => {
@@ -305,7 +305,7 @@ impl BackupWriter {
                         )
                         .await
                     {
-                        eprintln!("Error downloading .didx from previous manifest: {}", err);
+                        log::warn!("Error downloading .didx from previous manifest: {}", err);
                     }
                 }
                 _ => { /* do nothing */ }
-- 
2.39.2





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

* [pbs-devel] [PATCH pbs 2/2] fix #4761: pbs-client: backup_writer: improve error reporting
  2023-06-07 14:07 [pbs-devel] [PATCH pbs 1/2] pbs-client: backup-writer: use log::warn instead of eprintln! Maximiliano Sandoval
@ 2023-06-07 14:07 ` Maximiliano Sandoval
  2023-06-07 14:11   ` Maximiliano Sandoval
  2023-07-19  9:08 ` [pbs-devel] applied-series: [PATCH pbs 1/2] pbs-client: backup-writer: use log::warn instead of eprintln! Fabian Grünbichler
  1 sibling, 1 reply; 4+ messages in thread
From: Maximiliano Sandoval @ 2023-06-07 14:07 UTC (permalink / raw)
  To: pbs-devel

We check if the manifest contains an index for the requested archive, if
it does not we avoid downloading it and report a more helpful error
message.

Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
---
Best viewed with git diff -w.

 pbs-client/src/backup_writer.rs | 56 +++++++++++++++++++--------------
 1 file changed, 32 insertions(+), 24 deletions(-)

diff --git a/pbs-client/src/backup_writer.rs b/pbs-client/src/backup_writer.rs
index 2e18a060..03dbcab7 100644
--- a/pbs-client/src/backup_writer.rs
+++ b/pbs-client/src/backup_writer.rs
@@ -282,33 +282,41 @@ impl BackupWriter {
         let close_path = format!("{}_close", prefix);
 
         if let Some(manifest) = options.previous_manifest {
-            // try, but ignore errors
-            match ArchiveType::from_path(archive_name) {
-                Ok(ArchiveType::FixedIndex) => {
-                    if let Err(err) = self
-                        .download_previous_fixed_index(
-                            archive_name,
-                            &manifest,
-                            known_chunks.clone(),
-                        )
-                        .await
-                    {
-                        log::warn!("Error downloading .fidx from previous manifest: {}", err);
+            if !manifest
+                .files()
+                .iter()
+                .any(|file| file.filename == archive_name)
+            {
+                log::info!("There is no index with the name {archive_name}");
+            } else {
+                // try, but ignore errors
+                match ArchiveType::from_path(archive_name) {
+                    Ok(ArchiveType::FixedIndex) => {
+                        if let Err(err) = self
+                            .download_previous_fixed_index(
+                                archive_name,
+                                &manifest,
+                                known_chunks.clone(),
+                            )
+                            .await
+                        {
+                            log::warn!("Error downloading .fidx from previous manifest: {}", err);
+                        }
                     }
-                }
-                Ok(ArchiveType::DynamicIndex) => {
-                    if let Err(err) = self
-                        .download_previous_dynamic_index(
-                            archive_name,
-                            &manifest,
-                            known_chunks.clone(),
-                        )
-                        .await
-                    {
-                        log::warn!("Error downloading .didx from previous manifest: {}", err);
+                    Ok(ArchiveType::DynamicIndex) => {
+                        if let Err(err) = self
+                            .download_previous_dynamic_index(
+                                archive_name,
+                                &manifest,
+                                known_chunks.clone(),
+                            )
+                            .await
+                        {
+                            log::warn!("Error downloading .didx from previous manifest: {}", err);
+                        }
                     }
+                    _ => { /* do nothing */ }
                 }
-                _ => { /* do nothing */ }
             }
         }
 
-- 
2.39.2





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

* Re: [pbs-devel] [PATCH pbs 2/2] fix #4761: pbs-client: backup_writer: improve error reporting
  2023-06-07 14:07 ` [pbs-devel] [PATCH pbs 2/2] fix #4761: pbs-client: backup_writer: improve error reporting Maximiliano Sandoval
@ 2023-06-07 14:11   ` Maximiliano Sandoval
  0 siblings, 0 replies; 4+ messages in thread
From: Maximiliano Sandoval @ 2023-06-07 14:11 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion

I have put the wrong issue number in the commit title, it is actually 4591.

> On 07.06.2023 16:07 CEST Maximiliano Sandoval <m.sandoval@proxmox.com> wrote:
> 
>  
> We check if the manifest contains an index for the requested archive, if
> it does not we avoid downloading it and report a more helpful error
> message.
> 
> Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
> ---
> Best viewed with git diff -w.
> 
>  pbs-client/src/backup_writer.rs | 56 +++++++++++++++++++--------------
>  1 file changed, 32 insertions(+), 24 deletions(-)
> 
> diff --git a/pbs-client/src/backup_writer.rs b/pbs-client/src/backup_writer.rs
> index 2e18a060..03dbcab7 100644
> --- a/pbs-client/src/backup_writer.rs
> +++ b/pbs-client/src/backup_writer.rs
> @@ -282,33 +282,41 @@ impl BackupWriter {
>          let close_path = format!("{}_close", prefix);
>  
>          if let Some(manifest) = options.previous_manifest {
> -            // try, but ignore errors
> -            match ArchiveType::from_path(archive_name) {
> -                Ok(ArchiveType::FixedIndex) => {
> -                    if let Err(err) = self
> -                        .download_previous_fixed_index(
> -                            archive_name,
> -                            &manifest,
> -                            known_chunks.clone(),
> -                        )
> -                        .await
> -                    {
> -                        log::warn!("Error downloading .fidx from previous manifest: {}", err);
> +            if !manifest
> +                .files()
> +                .iter()
> +                .any(|file| file.filename == archive_name)
> +            {
> +                log::info!("There is no index with the name {archive_name}");
> +            } else {
> +                // try, but ignore errors
> +                match ArchiveType::from_path(archive_name) {
> +                    Ok(ArchiveType::FixedIndex) => {
> +                        if let Err(err) = self
> +                            .download_previous_fixed_index(
> +                                archive_name,
> +                                &manifest,
> +                                known_chunks.clone(),
> +                            )
> +                            .await
> +                        {
> +                            log::warn!("Error downloading .fidx from previous manifest: {}", err);
> +                        }
>                      }
> -                }
> -                Ok(ArchiveType::DynamicIndex) => {
> -                    if let Err(err) = self
> -                        .download_previous_dynamic_index(
> -                            archive_name,
> -                            &manifest,
> -                            known_chunks.clone(),
> -                        )
> -                        .await
> -                    {
> -                        log::warn!("Error downloading .didx from previous manifest: {}", err);
> +                    Ok(ArchiveType::DynamicIndex) => {
> +                        if let Err(err) = self
> +                            .download_previous_dynamic_index(
> +                                archive_name,
> +                                &manifest,
> +                                known_chunks.clone(),
> +                            )
> +                            .await
> +                        {
> +                            log::warn!("Error downloading .didx from previous manifest: {}", err);
> +                        }
>                      }
> +                    _ => { /* do nothing */ }
>                  }
> -                _ => { /* do nothing */ }
>              }
>          }
>  
> -- 
> 2.39.2
> 
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel




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

* [pbs-devel] applied-series: [PATCH pbs 1/2] pbs-client: backup-writer: use log::warn instead of eprintln!
  2023-06-07 14:07 [pbs-devel] [PATCH pbs 1/2] pbs-client: backup-writer: use log::warn instead of eprintln! Maximiliano Sandoval
  2023-06-07 14:07 ` [pbs-devel] [PATCH pbs 2/2] fix #4761: pbs-client: backup_writer: improve error reporting Maximiliano Sandoval
@ 2023-07-19  9:08 ` Fabian Grünbichler
  1 sibling, 0 replies; 4+ messages in thread
From: Fabian Grünbichler @ 2023-07-19  9:08 UTC (permalink / raw)
  To: Proxmox Backup Server development discussion

with the bug number fixed-up, and the (new) info message improved as
follow-up.

On June 7, 2023 4:07 pm, Maximiliano Sandoval wrote:
> The errors are not fatal so we only throw a warning.
> 
> Signed-off-by: Maximiliano Sandoval <m.sandoval@proxmox.com>
> ---
>  pbs-client/src/backup_writer.rs | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/pbs-client/src/backup_writer.rs b/pbs-client/src/backup_writer.rs
> index be6da2a6..2e18a060 100644
> --- a/pbs-client/src/backup_writer.rs
> +++ b/pbs-client/src/backup_writer.rs
> @@ -293,7 +293,7 @@ impl BackupWriter {
>                          )
>                          .await
>                      {
> -                        eprintln!("Error downloading .fidx from previous manifest: {}", err);
> +                        log::warn!("Error downloading .fidx from previous manifest: {}", err);
>                      }
>                  }
>                  Ok(ArchiveType::DynamicIndex) => {
> @@ -305,7 +305,7 @@ impl BackupWriter {
>                          )
>                          .await
>                      {
> -                        eprintln!("Error downloading .didx from previous manifest: {}", err);
> +                        log::warn!("Error downloading .didx from previous manifest: {}", err);
>                      }
>                  }
>                  _ => { /* do nothing */ }
> -- 
> 2.39.2
> 
> 
> 
> _______________________________________________
> pbs-devel mailing list
> pbs-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel
> 
> 
> 




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

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

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-07 14:07 [pbs-devel] [PATCH pbs 1/2] pbs-client: backup-writer: use log::warn instead of eprintln! Maximiliano Sandoval
2023-06-07 14:07 ` [pbs-devel] [PATCH pbs 2/2] fix #4761: pbs-client: backup_writer: improve error reporting Maximiliano Sandoval
2023-06-07 14:11   ` Maximiliano Sandoval
2023-07-19  9:08 ` [pbs-devel] applied-series: [PATCH pbs 1/2] pbs-client: backup-writer: use log::warn instead of eprintln! Fabian Grünbichler

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