From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from gate001.proxmox.com (gate001.proxmox.com [IPv6:2a0f:8001:1:32::40]) by lore.proxmox.com (Postfix) with ESMTPS id A7ACE1FF0E6 for ; Fri, 24 Jul 2026 11:44:11 +0200 (CEST) Received: from gate001.proxmox.com (localhost.localdomain [127.0.0.1]) by gate001.proxmox.com (Proxmox) with ESMTP id 34B872148A; Fri, 24 Jul 2026 11:44:11 +0200 (CEST) Date: Fri, 24 Jul 2026 11:43:34 +0200 From: Wolfgang Bumiller To: Arthur Bied-Charreton Subject: Re: [PATCH proxmox 10/29] notify: matcher: add InlineCalendarMatcher Message-ID: References: <20260709115716.299836-1-l.wagner@proxmox.com> <20260709115716.299836-11-l.wagner@proxmox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1784886184439 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.245 Adjusted score from AWL reputation of From: address DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment (newer systems) RCVD_IN_DNSWL_LOW -0.7 Sender listed at https://www.dnswl.org/, low trust SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Message-ID-Hash: EJAI77VOLV2LGLIPPZBMZCOQ77TJMVP5 X-Message-ID-Hash: EJAI77VOLV2LGLIPPZBMZCOQ77TJMVP5 X-MailFrom: w.bumiller@proxmox.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; loop; banned-address; emergency; member-moderation; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; digests; suspicious-header CC: Lukas Wagner , pbs-devel@lists.proxmox.com, pve-devel@lists.proxmox.com X-Mailman-Version: 3.3.10 Precedence: list List-Id: Proxmox Backup Server development discussion List-Help: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: On Fri, Jul 24, 2026 at 08:47:00AM +0200, Arthur Bied-Charreton wrote: > On Thu, Jul 09, 2026 at 01:56:57PM +0200, Lukas Wagner wrote: > > This move the inline serialization format into a new-type wrapper around > > CalendarMatcher. > > > > This allows us to keep the existing single-line serialization format for > > the old configuration keys (match-calendar), while deriving a regular > > Serializer for CalendarMatcher that will be used in the new expression > > based matcher. > > > one comment inline > > Signed-off-by: Lukas Wagner > > --- > > proxmox-notify/src/matcher/calendar.rs | 103 +++++++++++++++++++++---- > > proxmox-notify/src/matcher/mod.rs | 4 +- > > 2 files changed, 88 insertions(+), 19 deletions(-) > > > > diff --git a/proxmox-notify/src/matcher/calendar.rs b/proxmox-notify/src/matcher/calendar.rs > > index 970801c5..c6263eb5 100644 > > --- a/proxmox-notify/src/matcher/calendar.rs > > +++ b/proxmox-notify/src/matcher/calendar.rs > > @@ -1,49 +1,104 @@ > > use std::fmt; > > use std::str::FromStr; > > > > +use serde::{Deserialize, Serialize}; > > + > > use proxmox_time::DailyDuration; > > > > use crate::{Error, Notification}; > > > > use super::MatchDirective; > > > > -/// Match timestamp of the notification. > > +/// Convenience wrapper around [`DailyDuration`] that implements [`Serialize`] and > > +/// [`Deserialize`]. > > #[derive(Clone, Debug)] > > +struct DailyDurationWrapper { > > + schedule: String, > > + daily_duration: DailyDuration, > > +} > > + > > +impl fmt::Display for DailyDurationWrapper { > > + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { > > + f.write_str(self.as_str()) > > + } > > +} > > + > > +impl FromStr for DailyDurationWrapper { > > + type Err = Error; > > + > > + fn from_str(s: &str) -> Result { > > + let daily_duration = proxmox_time::parse_daily_duration(s) > we might wanna trim s here? this fails if s has a leading whitespace, > which is a code path reachable via the UI (see [0]), and even if/when > that is fixed, i feel like it would be nicer to not fail to parse > because of surrounding whitespace (could also be done at another level > of the call stack), since that would still be reachable via pvesh etc. I would avoid allowing white space in the `FromStr` implementation and prefer to keep the responsibility here with the caller, to stay consistent with other `FromStr` implementations. (Eg. the std lib fails to parse " 33" or "33 " as an `usize`.) > > [0] https://lore.proxmox.com/pve-devel/20260709115716.299836-1-l.wagner@proxmox.com/T/#mad9286504ecf2264333be5abe8b9e1f4966aeaeb > > + .map_err(|e| Error::Generic(format!("could not parse schedule: {e}")))?; > > + > > + Ok(Self { > > + daily_duration, > > + schedule: s.to_string(), > > + }) > > + } > > +} > > + > > +impl DailyDurationWrapper { > > + fn as_str(&self) -> &str { > > + &self.schedule > > + } > > +} > > + > > +proxmox_serde::forward_deserialize_to_from_str!(DailyDurationWrapper); > > +proxmox_serde::forward_serialize_to_display!(DailyDurationWrapper); > > + > > +/// Match timestamp of the notification. > > +#[derive(Clone, Debug, Serialize, Deserialize)] > > +#[serde(rename_all = "kebab-case")] > > pub struct CalendarMatcher { > > - schedule: DailyDuration, > > - original: String, > > + schedule: DailyDurationWrapper, > > } > > > > impl MatchDirective for CalendarMatcher { > > fn matches(&self, notification: &Notification) -> Result { > > self.schedule > > + .daily_duration > > .time_match(notification.metadata.timestamp, false) > > .map_err(|err| Error::Generic(format!("could not match timestamp: {err}"))) > > } > > } > > > > -impl fmt::Display for CalendarMatcher { > > - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { > > - f.write_str(&self.original) > > +/// Match the timestamp of a notification. > > +/// > > +/// This is a wrapper that serializes into an inline format that can be used > > +/// in a section config key-value pair. > > +#[derive(Clone, Debug)] > > +pub struct InlineCalendarMatcher(CalendarMatcher); > > + > > +impl MatchDirective for InlineCalendarMatcher { > > + fn matches(&self, notification: &Notification) -> Result { > > + self.0.matches(notification) > > } > > } > > > > -impl FromStr for CalendarMatcher { > > +impl fmt::Display for InlineCalendarMatcher { > > + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { > > + f.write_str(self.0.schedule.as_str()) > > + } > > +} > > + > > +impl FromStr for InlineCalendarMatcher { > > type Err = Error; > > > > fn from_str(s: &str) -> Result { > > - let schedule = proxmox_time::parse_daily_duration(s) > > - .map_err(|e| Error::Generic(format!("could not parse schedule: {e}")))?; > > - > > - Ok(Self { > > - schedule, > > - original: s.to_string(), > > - }) > > + Ok(Self(CalendarMatcher { > > + schedule: s.parse()?, > > + })) > > } > > } > > > > -proxmox_serde::forward_deserialize_to_from_str!(CalendarMatcher); > > -proxmox_serde::forward_serialize_to_display!(CalendarMatcher); > > +impl InlineCalendarMatcher { > > + pub fn into_inner(self) -> CalendarMatcher { > > + self.0 > > + } > > +} > > + > > +proxmox_serde::forward_deserialize_to_from_str!(InlineCalendarMatcher); > > +proxmox_serde::forward_serialize_to_display!(InlineCalendarMatcher); > > > > #[cfg(test)] > > mod test { > > @@ -63,7 +118,21 @@ mod test { > > > > // Match on a wide rage to avoid issues when running this test case > > // in a different time zone. > > - let matcher: CalendarMatcher = "thu..sat 0-23".parse().unwrap(); > > + let matcher: InlineCalendarMatcher = "thu..sat 0-23".parse().unwrap(); > > assert!(matcher.matches(¬ification).unwrap()); > > } > > + > > + #[test] > > + fn test_calendar_matcher_de_ser_roundtrip() { > > + let calendar_matcher = "{ \"schedule\": \"thu..sat 0-23\" }"; > > + > > + let calendar_matcher: CalendarMatcher = serde_json::from_str(calendar_matcher).unwrap(); > > + > > + let s = serde_json::to_string(&calendar_matcher).unwrap(); > > + let m: CalendarMatcher = serde_json::from_str(&s).unwrap(); > > + > > + let a = InlineCalendarMatcher(m); > > + > > + assert_eq!(a.to_string(), "thu..sat 0-23"); > > + } > > } > > diff --git a/proxmox-notify/src/matcher/mod.rs b/proxmox-notify/src/matcher/mod.rs > > index 0267a2b9..a5d53d8a 100644 > > --- a/proxmox-notify/src/matcher/mod.rs > > +++ b/proxmox-notify/src/matcher/mod.rs > > @@ -14,7 +14,7 @@ pub mod calendar; > > pub mod field; > > pub mod severity; > > > > -use calendar::CalendarMatcher; > > +use calendar::InlineCalendarMatcher; > > use field::InlineFieldMatcher; > > use severity::InlineSeverityMatcher; > > > > @@ -112,7 +112,7 @@ pub struct MatcherConfig { > > /// List of matched severity levels. > > #[serde(default, skip_serializing_if = "Vec::is_empty")] > > #[updater(serde(skip_serializing_if = "Option::is_none"))] > > - pub match_calendar: Vec, > > + pub match_calendar: Vec, > > /// Decide if 'all' or 'any' match statements must match. > > #[serde(skip_serializing_if = "Option::is_none")] > > pub mode: Option, > > -- > > 2.47.3