* [pbs-devel] [PATCH proxmox-backup] SnapshotVerifyState: use enum for state
@ 2020-09-15 8:19 Stefan Reiter
2020-09-15 10:50 ` Dietmar Maurer
2020-09-15 11:05 ` [pbs-devel] applied: " Dietmar Maurer
0 siblings, 2 replies; 4+ messages in thread
From: Stefan Reiter @ 2020-09-15 8:19 UTC (permalink / raw)
To: pbs-devel
Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
---
src/api2/backup.rs | 8 +++-----
src/api2/backup/environment.rs | 4 ++--
src/api2/types/mod.rs | 17 ++++++++++++++---
src/backup/verify.rs | 7 +++----
4 files changed, 22 insertions(+), 14 deletions(-)
diff --git a/src/api2/backup.rs b/src/api2/backup.rs
index 371260dc..66854d4c 100644
--- a/src/api2/backup.rs
+++ b/src/api2/backup.rs
@@ -120,11 +120,9 @@ async move {
let verify = manifest.unprotected["verify_state"].clone();
match serde_json::from_value::<SnapshotVerifyState>(verify) {
Ok(verify) => {
- if verify.state != "ok" {
- // verify failed, treat as if no previous backup exists
- None
- } else {
- Some(info)
+ match verify.state {
+ VerifyState::Ok => Some(info),
+ VerifyState::Failed => None,
}
},
Err(_) => {
diff --git a/src/api2/backup/environment.rs b/src/api2/backup/environment.rs
index 22b96c22..d515bf30 100644
--- a/src/api2/backup/environment.rs
+++ b/src/api2/backup/environment.rs
@@ -9,7 +9,7 @@ use proxmox::tools::digest_to_hex;
use proxmox::tools::fs::{replace_file, CreateOptions};
use proxmox::api::{RpcEnvironment, RpcEnvironmentType};
-use crate::api2::types::{Userid, SnapshotVerifyState};
+use crate::api2::types::{Userid, SnapshotVerifyState, VerifyState};
use crate::backup::*;
use crate::server::WorkerTask;
use crate::server::formatter::*;
@@ -466,7 +466,7 @@ impl BackupEnvironment {
let mark_msg = if let Some(ref last_backup) = self.last_backup {
let last_dir = &last_backup.backup_dir;
let verify_state = SnapshotVerifyState {
- state: "failed".to_owned(),
+ state: VerifyState::Failed,
upid: self.worker.upid().clone(),
};
diff --git a/src/api2/types/mod.rs b/src/api2/types/mod.rs
index e29a7e37..5495d693 100644
--- a/src/api2/types/mod.rs
+++ b/src/api2/types/mod.rs
@@ -380,13 +380,24 @@ pub struct GroupListItem {
pub owner: Option<Userid>,
}
+#[api()]
+#[derive(Debug, Copy, Clone, PartialEq, Serialize, Deserialize)]
+#[serde(rename_all = "lowercase")]
+/// Result of a verify operation.
+pub enum VerifyState {
+ /// Verification was successful
+ Ok,
+ /// Verification reported one or more errors
+ Failed,
+}
+
#[api(
properties: {
upid: {
schema: UPID_SCHEMA
},
state: {
- type: String
+ type: VerifyState
},
},
)]
@@ -395,8 +406,8 @@ pub struct GroupListItem {
pub struct SnapshotVerifyState {
/// UPID of the verify task
pub upid: UPID,
- /// State of the verification. "failed" or "ok"
- pub state: String,
+ /// State of the verification. Enum.
+ pub state: VerifyState,
}
#[api(
diff --git a/src/backup/verify.rs b/src/backup/verify.rs
index b75cdfb8..1fad6187 100644
--- a/src/backup/verify.rs
+++ b/src/backup/verify.rs
@@ -283,7 +283,7 @@ pub fn verify_backup_dir(
let mut error_count = 0;
- let mut verify_result = "ok";
+ let mut verify_result = VerifyState::Ok;
for info in manifest.files() {
let result = proxmox::try_block!({
worker.log(format!(" check {}", info.filename));
@@ -316,20 +316,19 @@ pub fn verify_backup_dir(
if let Err(err) = result {
worker.log(format!("verify {}:{}/{} failed: {}", datastore.name(), backup_dir, info.filename, err));
error_count += 1;
- verify_result = "failed";
+ verify_result = VerifyState::Failed;
}
}
let verify_state = SnapshotVerifyState {
- state: verify_result.to_string(),
+ state: verify_result,
upid: worker.upid().clone(),
};
manifest.unprotected["verify_state"] = serde_json::to_value(verify_state)?;
datastore.store_manifest(&backup_dir, serde_json::to_value(manifest)?)
.map_err(|err| format_err!("unable to store manifest blob - {}", err))?;
-
Ok(error_count == 0)
}
--
2.20.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [pbs-devel] [PATCH proxmox-backup] SnapshotVerifyState: use enum for state
2020-09-15 8:19 [pbs-devel] [PATCH proxmox-backup] SnapshotVerifyState: use enum for state Stefan Reiter
@ 2020-09-15 10:50 ` Dietmar Maurer
2020-09-15 11:54 ` Stefan Reiter
2020-09-15 11:05 ` [pbs-devel] applied: " Dietmar Maurer
1 sibling, 1 reply; 4+ messages in thread
From: Dietmar Maurer @ 2020-09-15 10:50 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Stefan Reiter
Why not a bool ?
> On 09/15/2020 10:19 AM Stefan Reiter <s.reiter@proxmox.com> wrote:
>
>
> Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
> ---
> src/api2/backup.rs | 8 +++-----
> src/api2/backup/environment.rs | 4 ++--
> src/api2/types/mod.rs | 17 ++++++++++++++---
> src/backup/verify.rs | 7 +++----
> 4 files changed, 22 insertions(+), 14 deletions(-)
>
> diff --git a/src/api2/backup.rs b/src/api2/backup.rs
> index 371260dc..66854d4c 100644
> --- a/src/api2/backup.rs
> +++ b/src/api2/backup.rs
> @@ -120,11 +120,9 @@ async move {
> let verify = manifest.unprotected["verify_state"].clone();
> match serde_json::from_value::<SnapshotVerifyState>(verify) {
> Ok(verify) => {
> - if verify.state != "ok" {
> - // verify failed, treat as if no previous backup exists
> - None
> - } else {
> - Some(info)
> + match verify.state {
> + VerifyState::Ok => Some(info),
> + VerifyState::Failed => None,
> }
> },
> Err(_) => {
> diff --git a/src/api2/backup/environment.rs b/src/api2/backup/environment.rs
> index 22b96c22..d515bf30 100644
> --- a/src/api2/backup/environment.rs
> +++ b/src/api2/backup/environment.rs
> @@ -9,7 +9,7 @@ use proxmox::tools::digest_to_hex;
> use proxmox::tools::fs::{replace_file, CreateOptions};
> use proxmox::api::{RpcEnvironment, RpcEnvironmentType};
>
> -use crate::api2::types::{Userid, SnapshotVerifyState};
> +use crate::api2::types::{Userid, SnapshotVerifyState, VerifyState};
> use crate::backup::*;
> use crate::server::WorkerTask;
> use crate::server::formatter::*;
> @@ -466,7 +466,7 @@ impl BackupEnvironment {
> let mark_msg = if let Some(ref last_backup) = self.last_backup {
> let last_dir = &last_backup.backup_dir;
> let verify_state = SnapshotVerifyState {
> - state: "failed".to_owned(),
> + state: VerifyState::Failed,
> upid: self.worker.upid().clone(),
> };
>
> diff --git a/src/api2/types/mod.rs b/src/api2/types/mod.rs
> index e29a7e37..5495d693 100644
> --- a/src/api2/types/mod.rs
> +++ b/src/api2/types/mod.rs
> @@ -380,13 +380,24 @@ pub struct GroupListItem {
> pub owner: Option<Userid>,
> }
>
> +#[api()]
> +#[derive(Debug, Copy, Clone, PartialEq, Serialize, Deserialize)]
> +#[serde(rename_all = "lowercase")]
> +/// Result of a verify operation.
> +pub enum VerifyState {
> + /// Verification was successful
> + Ok,
> + /// Verification reported one or more errors
> + Failed,
> +}
> +
> #[api(
> properties: {
> upid: {
> schema: UPID_SCHEMA
> },
> state: {
> - type: String
> + type: VerifyState
> },
> },
> )]
> @@ -395,8 +406,8 @@ pub struct GroupListItem {
> pub struct SnapshotVerifyState {
> /// UPID of the verify task
> pub upid: UPID,
> - /// State of the verification. "failed" or "ok"
> - pub state: String,
> + /// State of the verification. Enum.
> + pub state: VerifyState,
> }
>
> #[api(
> diff --git a/src/backup/verify.rs b/src/backup/verify.rs
> index b75cdfb8..1fad6187 100644
> --- a/src/backup/verify.rs
> +++ b/src/backup/verify.rs
> @@ -283,7 +283,7 @@ pub fn verify_backup_dir(
>
> let mut error_count = 0;
>
> - let mut verify_result = "ok";
> + let mut verify_result = VerifyState::Ok;
> for info in manifest.files() {
> let result = proxmox::try_block!({
> worker.log(format!(" check {}", info.filename));
> @@ -316,20 +316,19 @@ pub fn verify_backup_dir(
> if let Err(err) = result {
> worker.log(format!("verify {}:{}/{} failed: {}", datastore.name(), backup_dir, info.filename, err));
> error_count += 1;
> - verify_result = "failed";
> + verify_result = VerifyState::Failed;
> }
>
> }
>
> let verify_state = SnapshotVerifyState {
> - state: verify_result.to_string(),
> + state: verify_result,
> upid: worker.upid().clone(),
> };
> manifest.unprotected["verify_state"] = serde_json::to_value(verify_state)?;
> datastore.store_manifest(&backup_dir, serde_json::to_value(manifest)?)
> .map_err(|err| format_err!("unable to store manifest blob - {}", err))?;
>
> -
> Ok(error_count == 0)
> }
>
> --
> 2.20.1
>
>
>
> _______________________________________________
> 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: [PATCH proxmox-backup] SnapshotVerifyState: use enum for state
2020-09-15 8:19 [pbs-devel] [PATCH proxmox-backup] SnapshotVerifyState: use enum for state Stefan Reiter
2020-09-15 10:50 ` Dietmar Maurer
@ 2020-09-15 11:05 ` Dietmar Maurer
1 sibling, 0 replies; 4+ messages in thread
From: Dietmar Maurer @ 2020-09-15 11:05 UTC (permalink / raw)
To: Proxmox Backup Server development discussion, Stefan Reiter
applied
> On 09/15/2020 10:19 AM Stefan Reiter <s.reiter@proxmox.com> wrote:
>
>
> Signed-off-by: Stefan Reiter <s.reiter@proxmox.com>
> ---
> src/api2/backup.rs | 8 +++-----
> src/api2/backup/environment.rs | 4 ++--
> src/api2/types/mod.rs | 17 ++++++++++++++---
> src/backup/verify.rs | 7 +++----
> 4 files changed, 22 insertions(+), 14 deletions(-)
>
> diff --git a/src/api2/backup.rs b/src/api2/backup.rs
> index 371260dc..66854d4c 100644
> --- a/src/api2/backup.rs
> +++ b/src/api2/backup.rs
> @@ -120,11 +120,9 @@ async move {
> let verify = manifest.unprotected["verify_state"].clone();
> match serde_json::from_value::<SnapshotVerifyState>(verify) {
> Ok(verify) => {
> - if verify.state != "ok" {
> - // verify failed, treat as if no previous backup exists
> - None
> - } else {
> - Some(info)
> + match verify.state {
> + VerifyState::Ok => Some(info),
> + VerifyState::Failed => None,
> }
> },
> Err(_) => {
> diff --git a/src/api2/backup/environment.rs b/src/api2/backup/environment.rs
> index 22b96c22..d515bf30 100644
> --- a/src/api2/backup/environment.rs
> +++ b/src/api2/backup/environment.rs
> @@ -9,7 +9,7 @@ use proxmox::tools::digest_to_hex;
> use proxmox::tools::fs::{replace_file, CreateOptions};
> use proxmox::api::{RpcEnvironment, RpcEnvironmentType};
>
> -use crate::api2::types::{Userid, SnapshotVerifyState};
> +use crate::api2::types::{Userid, SnapshotVerifyState, VerifyState};
> use crate::backup::*;
> use crate::server::WorkerTask;
> use crate::server::formatter::*;
> @@ -466,7 +466,7 @@ impl BackupEnvironment {
> let mark_msg = if let Some(ref last_backup) = self.last_backup {
> let last_dir = &last_backup.backup_dir;
> let verify_state = SnapshotVerifyState {
> - state: "failed".to_owned(),
> + state: VerifyState::Failed,
> upid: self.worker.upid().clone(),
> };
>
> diff --git a/src/api2/types/mod.rs b/src/api2/types/mod.rs
> index e29a7e37..5495d693 100644
> --- a/src/api2/types/mod.rs
> +++ b/src/api2/types/mod.rs
> @@ -380,13 +380,24 @@ pub struct GroupListItem {
> pub owner: Option<Userid>,
> }
>
> +#[api()]
> +#[derive(Debug, Copy, Clone, PartialEq, Serialize, Deserialize)]
> +#[serde(rename_all = "lowercase")]
> +/// Result of a verify operation.
> +pub enum VerifyState {
> + /// Verification was successful
> + Ok,
> + /// Verification reported one or more errors
> + Failed,
> +}
> +
> #[api(
> properties: {
> upid: {
> schema: UPID_SCHEMA
> },
> state: {
> - type: String
> + type: VerifyState
> },
> },
> )]
> @@ -395,8 +406,8 @@ pub struct GroupListItem {
> pub struct SnapshotVerifyState {
> /// UPID of the verify task
> pub upid: UPID,
> - /// State of the verification. "failed" or "ok"
> - pub state: String,
> + /// State of the verification. Enum.
> + pub state: VerifyState,
> }
>
> #[api(
> diff --git a/src/backup/verify.rs b/src/backup/verify.rs
> index b75cdfb8..1fad6187 100644
> --- a/src/backup/verify.rs
> +++ b/src/backup/verify.rs
> @@ -283,7 +283,7 @@ pub fn verify_backup_dir(
>
> let mut error_count = 0;
>
> - let mut verify_result = "ok";
> + let mut verify_result = VerifyState::Ok;
> for info in manifest.files() {
> let result = proxmox::try_block!({
> worker.log(format!(" check {}", info.filename));
> @@ -316,20 +316,19 @@ pub fn verify_backup_dir(
> if let Err(err) = result {
> worker.log(format!("verify {}:{}/{} failed: {}", datastore.name(), backup_dir, info.filename, err));
> error_count += 1;
> - verify_result = "failed";
> + verify_result = VerifyState::Failed;
> }
>
> }
>
> let verify_state = SnapshotVerifyState {
> - state: verify_result.to_string(),
> + state: verify_result,
> upid: worker.upid().clone(),
> };
> manifest.unprotected["verify_state"] = serde_json::to_value(verify_state)?;
> datastore.store_manifest(&backup_dir, serde_json::to_value(manifest)?)
> .map_err(|err| format_err!("unable to store manifest blob - {}", err))?;
>
> -
> Ok(error_count == 0)
> }
>
> --
> 2.20.1
>
>
>
> _______________________________________________
> 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
* Re: [pbs-devel] [PATCH proxmox-backup] SnapshotVerifyState: use enum for state
2020-09-15 10:50 ` Dietmar Maurer
@ 2020-09-15 11:54 ` Stefan Reiter
0 siblings, 0 replies; 4+ messages in thread
From: Stefan Reiter @ 2020-09-15 11:54 UTC (permalink / raw)
To: Dietmar Maurer, Proxmox Backup Server development discussion
On 9/15/20 12:50 PM, Dietmar Maurer wrote:
> Why not a bool ?
>
More future proof I suppose, also keeps existing entries valid (name
deserialization).
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-09-15 11:54 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-15 8:19 [pbs-devel] [PATCH proxmox-backup] SnapshotVerifyState: use enum for state Stefan Reiter
2020-09-15 10:50 ` Dietmar Maurer
2020-09-15 11:54 ` Stefan Reiter
2020-09-15 11:05 ` [pbs-devel] applied: " Dietmar Maurer
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