* [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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox