From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [212.224.123.68]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by lists.proxmox.com (Postfix) with ESMTPS id 5BE5A61DE6 for ; Tue, 15 Sep 2020 12:51:45 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 5168519A86 for ; Tue, 15 Sep 2020 12:51:15 +0200 (CEST) Received: from proxmox-new.maurer-it.com (proxmox-new.maurer-it.com [212.186.127.180]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by firstgate.proxmox.com (Proxmox) with ESMTPS id 487AC19A79 for ; Tue, 15 Sep 2020 12:51:14 +0200 (CEST) Received: from proxmox-new.maurer-it.com (localhost.localdomain [127.0.0.1]) by proxmox-new.maurer-it.com (Proxmox) with ESMTP id 107BB44C31 for ; Tue, 15 Sep 2020 12:51:14 +0200 (CEST) Date: Tue, 15 Sep 2020 12:50:27 +0200 (CEST) From: Dietmar Maurer To: Proxmox Backup Server development discussion , Stefan Reiter Message-ID: <364580372.105.1600167028187@webmail.proxmox.com> In-Reply-To: <20200915081923.11337-1-s.reiter@proxmox.com> References: <20200915081923.11337-1-s.reiter@proxmox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Priority: 3 Importance: Normal X-Mailer: Open-Xchange Mailer v7.10.3-Rev22 X-Originating-Client: open-xchange-appsuite X-SPAM-LEVEL: Spam detection results: 0 AWL 0.087 Adjusted score from AWL reputation of From: address KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_DNSWL_MED -2.3 Sender listed at https://www.dnswl.org/, medium trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record URIBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to URIBL was blocked. See http://wiki.apache.org/spamassassin/DnsBlocklists#dnsbl-block for more information. [verify.rs, proxmox.com, environment.rs, backup.rs, mod.rs] Subject: Re: [pbs-devel] [PATCH proxmox-backup] SnapshotVerifyState: use enum for state X-BeenThere: pbs-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Backup Server development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 15 Sep 2020 10:51:45 -0000 Why not a bool ? > On 09/15/2020 10:19 AM Stefan Reiter wrote: > > > Signed-off-by: Stefan Reiter > --- > 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::(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, > } > > +#[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