From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id EB2651FF2C6 for ; Tue, 9 Jul 2024 14:09:43 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 1F09B1EFAF; Tue, 9 Jul 2024 14:10:06 +0200 (CEST) Date: Tue, 9 Jul 2024 14:09:32 +0200 (CEST) From: Christian Ebner To: Proxmox Backup Server development discussion , Gabriel Goller Message-ID: <210562830.1744.1720526972589@webmail.proxmox.com> In-Reply-To: <20240709093737.zvwlhvneuhxlxcrw@luna.proxmox.com> References: <20240704085559.132116-1-c.ebner@proxmox.com> <20240704085559.132116-3-c.ebner@proxmox.com> <20240709093737.zvwlhvneuhxlxcrw@luna.proxmox.com> MIME-Version: 1.0 X-Priority: 3 Importance: Normal X-Mailer: Open-Xchange Mailer v7.10.6-Rev66 X-Originating-Client: open-xchange-appsuite X-SPAM-LEVEL: Spam detection results: 0 AWL 0.021 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment 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. [self.name, datastore.rs] Subject: Re: [pbs-devel] [PATCH proxmox-backup 2/4] api types: introduce `BackupArchiveName` type 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: , Reply-To: Proxmox Backup Server development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pbs-devel-bounces@lists.proxmox.com Sender: "pbs-devel" > On 09.07.2024 11:37 CEST Gabriel Goller wrote: > > > Doesn't apply on master anymore, but here some quick things I noticed > skimming over. > > On 04.07.2024 10:55, Christian Ebner wrote: > >diff --git a/pbs-api-types/src/datastore.rs b/pbs-api-types/src/datastore.rs > >index b63e7c2ff..16c3eef59 100644 > >--- a/pbs-api-types/src/datastore.rs > >+++ b/pbs-api-types/src/datastore.rs > >+impl std::fmt::Display for BackupArchiveName { > > I think we can import the std::* traits here (and below as well.) Okay, fine by me, will use the already imported fmt module for traits located in there instead, to be in line with the current code and import other traits at the beginning. > > >+ fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { > >+ write!(f, "{name}", name = self.name) > >+ } > >+} > >+ > >+serde_plain::derive_deserialize_from_fromstr!(BackupArchiveName, "archive name"); > >+ > >+impl std::str::FromStr for BackupArchiveName { > >+ type Err = Error; > >+ > >+ fn from_str(name: &str) -> Result { > >+ Self::try_from(name) > >+ } > >+} > >+ > >+serde_plain::derive_serialize_from_display!(BackupArchiveName); > >+ > >+impl std::convert::TryFrom<&str> for BackupArchiveName { > >+ type Error = anyhow::Error; > >+ > >+ fn try_from(value: &str) -> Result { > >+ let (name, ty) = Self::parse_archive_type(value)?; > >+ Ok(Self { name, ty }) > >+ } > >+} > >+ > >+impl std::convert::AsRef for BackupArchiveName { > >+ fn as_ref(&self) -> &str { > >+ &self.name > >+ } > >+} > >+ > >+impl BackupArchiveName { > >+ pub fn from_path(path: impl AsRef) -> Result { > >+ let path = path.as_ref(); > >+ let file_name = path > >+ .file_name() > >+ .ok_or_else(|| format_err!("invalid archive name"))?; > >+ let name = file_name > >+ .to_str() > >+ .ok_or_else(|| format_err!("archive name not valid UTF-8"))?; > >+ > >+ Self::try_from(name) > >+ } > >+ > >+ pub fn archive_type(&self) -> ArchiveType { > >+ self.ty.clone() > >+ } > >+ > >+ pub fn ends_with(&self, postfix: &str) -> bool { > >+ self.name.ends_with(postfix) > >+ } > >+ > >+ pub fn has_pxar_filename_extension(&self) -> bool { > >+ self.name.ends_with(".pxar.didx") > >+ || self.name.ends_with(".mpxar.didx") > >+ || self.name.ends_with(".ppxar.didx") > >+ } > >+ > >+ pub fn without_type_extension(&self) -> String { > >+ match self.ty { > >+ ArchiveType::DynamicIndex => self.name.strip_suffix(".didx").unwrap().into(), > >+ ArchiveType::FixedIndex => self.name.strip_suffix(".fidx").unwrap().into(), > >+ ArchiveType::Blob => self.name.strip_suffix(".blob").unwrap().into(), > >+ } > >+ } > >+ > >+ fn parse_archive_type(archive_name: &str) -> Result<(String, ArchiveType), Error> { > >+ if archive_name.ends_with(".didx") > >+ || archive_name.ends_with(".fidx") > >+ || archive_name.ends_with(".blob") > >+ { > >+ Ok((archive_name.into(), ArchiveType::from_path(archive_name)?)) > >+ } else if archive_name.ends_with(".pxar") > >+ || archive_name.ends_with(".mpxar") > >+ || archive_name.ends_with(".ppxar") > >+ { > >+ Ok((format!("{}.didx", archive_name), ArchiveType::DynamicIndex)) > > `archive_name` can be inlined here (below as well.) Will be inlined in v2. > > >+ } else if archive_name.ends_with(".img") { > >+ Ok((format!("{}.fidx", archive_name), ArchiveType::FixedIndex)) > >+ } else { > >+ Ok((format!("{}.blob", archive_name), ArchiveType::Blob)) > >+ } > >+ } > >+} > >+ > >+impl ApiType for BackupArchiveName { > >+ const API_SCHEMA: Schema = BACKUP_ARCHIVE_NAME_SCHEMA; > >+} > > Everything quite straightforward, but we could throw in some unit-tests > as it won't do us any harm. Sure, will add tests covering the relevant cases. _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel