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 0CE231FF2C6 for ; Tue, 9 Jul 2024 11:37:22 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 5FD931BD89; Tue, 9 Jul 2024 11:37:42 +0200 (CEST) Date: Tue, 9 Jul 2024 11:37:37 +0200 From: Gabriel Goller To: Proxmox Backup Server development discussion Message-ID: <20240709093737.zvwlhvneuhxlxcrw@luna.proxmox.com> References: <20240704085559.132116-1-c.ebner@proxmox.com> <20240704085559.132116-3-c.ebner@proxmox.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20240704085559.132116-3-c.ebner@proxmox.com> X-SPAM-LEVEL: Spam detection results: 0 AWL -0.054 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. [datastore.rs, self.name] 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-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: pbs-devel-bounces@lists.proxmox.com Sender: "pbs-devel" 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.) >+ 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.) >+ } 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. _______________________________________________ pbs-devel mailing list pbs-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pbs-devel