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 9C5161FF15E for ; Mon, 24 Nov 2025 10:55:28 +0100 (CET) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id 3F2291D895; Mon, 24 Nov 2025 10:55:39 +0100 (CET) Message-ID: Date: Mon, 24 Nov 2025 10:55:05 +0100 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Beta To: Shannon Sterz References: <20251117125041.1931382-1-d.csapak@proxmox.com> <20251117125041.1931382-3-d.csapak@proxmox.com> Content-Language: en-US From: Dominik Csapak In-Reply-To: X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1763978070978 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.028 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 RCVD_IN_VALIDITY_CERTIFIED_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_RPBL_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. RCVD_IN_VALIDITY_SAFE_BLOCKED 0.001 ADMINISTRATOR NOTICE: The query to Validity was blocked. See https://knowledge.validity.com/hc/en-us/articles/20961730681243 for more information. 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. [views.rs, tests.rs] Subject: Re: [pdm-devel] [PATCH datacenter-manager v3 02/18] lib: api-types: add 'layout' property to ViewConfig X-BeenThere: pdm-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Datacenter Manager development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox Datacenter Manager development discussion Cc: Proxmox Datacenter Manager development discussion Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: pdm-devel-bounces@lists.proxmox.com Sender: "pdm-devel" On 11/17/25 3:57 PM, Shannon Sterz wrote: > On Mon Nov 17, 2025 at 1:44 PM CET, Dominik Csapak wrote: >> this is a simple string that holds the layout json. We can't currently >> add it as a normal property with 'correct' api types, since we want to >> use enum features that are not available with the api macro. >> >> Signed-off-by: Dominik Csapak >> --- >> lib/pdm-api-types/src/views.rs | 12 +++++++++++- >> server/src/views/tests.rs | 15 +++++++++++++++ >> 2 files changed, 26 insertions(+), 1 deletion(-) >> >> diff --git a/lib/pdm-api-types/src/views.rs b/lib/pdm-api-types/src/views.rs >> index ef39cc62..4b837384 100644 >> --- a/lib/pdm-api-types/src/views.rs >> +++ b/lib/pdm-api-types/src/views.rs >> @@ -47,7 +47,10 @@ pub const FILTER_RULE_LIST_SCHEMA: Schema = >> "exclude": { >> schema: FILTER_RULE_LIST_SCHEMA, >> optional: true, >> - } >> + }, >> + layout: { >> + optional: true, >> + }, >> } >> )] >> #[derive(Clone, Debug, Default, Deserialize, Serialize, Updater, PartialEq)] >> @@ -67,6 +70,13 @@ pub struct ViewConfig { >> #[serde(default, skip_serializing_if = "Vec::is_empty")] >> #[updater(serde(skip_serializing_if = "Option::is_none"))] >> pub exclude: Vec, >> + >> + // we can't currently describe this with the 'api' macro so save >> + // it simply as a string and check it in the add/update call > > one question: shouldn't this be `Value` instead? right now this needs to > be a string that is valid json. meaning that the api expects a json body > that contains `layout` as a string, with that string being escaped json > again. > > with `Value` this could be the json directly, avoiding having to escape > the json string. this would also make it easier to describe this later > on via the api macro if i'm not mistaken. the last commit in this series > could then also be dropped. > > what do you think? had a short look and it seems like that we can't currently combine the Updater derivation with Value since there is no impl of UpdaterType/ApiType for Value and we can't provide one. i shortly tried to implment a wrapper type, but then we run into the issue that we'd have to impl ApiType for that too and the fact that we can't currently represent the Layout in a schema is the whole point of the string workaround here. The only thing that could work here is to simply define an ObjectSchema with no properties and 'additional_properties' to true. I'll try that and see if it makes the situation better. > >> + /// The configured layout, encoded as json >> + #[serde(default, skip_serializing_if = "String::is_empty")] >> + #[updater(serde(skip_serializing_if = "Option::is_none"))] >> + pub layout: String, >> } >> >> #[derive(Clone, Debug, Deserialize, Serialize, PartialEq)] >> diff --git a/server/src/views/tests.rs b/server/src/views/tests.rs >> index 030b7994..c015acd2 100644 >> --- a/server/src/views/tests.rs >> +++ b/server/src/views/tests.rs >> @@ -184,6 +184,7 @@ fn include_exclude_remotes() { >> FilterRule::Remote("remote-b".into()), >> FilterRule::Remote("remote-c".into()), >> ], >> + layout: String::new(), >> }; >> run_test( >> config.clone(), >> @@ -327,6 +328,7 @@ fn include_exclude_type() { >> id: "exclude-resource-type".into(), >> include: vec![FilterRule::ResourceType(ResourceType::PveQemu)], >> exclude: vec![FilterRule::ResourceType(ResourceType::PveStorage)], >> + layout: String::new(), >> }, >> &[ >> ( >> @@ -355,6 +357,7 @@ fn include_exclude_tags() { >> FilterRule::Tag("tag2".to_string()), >> ], >> exclude: vec![FilterRule::Tag("tag3".to_string())], >> + layout: String::new(), >> }, >> &[ >> ( >> @@ -400,6 +403,7 @@ fn include_exclude_resource_pool() { >> FilterRule::ResourcePool("pool2".to_string()), >> ], >> exclude: vec![FilterRule::ResourcePool("pool2".to_string())], >> + layout: String::new(), >> }, >> &[ >> ( >> @@ -449,6 +453,7 @@ fn include_exclude_resource_id() { >> FilterRule::ResourceId("remote/otherremote/guest/101".to_string()), >> FilterRule::ResourceId(format!("remote/{REMOTE}/storage/{NODE}/otherstorage")), >> ], >> + layout: String::new(), >> }, >> &[ >> ( >> @@ -495,6 +500,7 @@ fn node_included() { >> FilterRule::ResourceId("remote/someremote/node/test".to_string()), >> ], >> exclude: vec![FilterRule::Remote("remote-b".to_string())], >> + layout: String::new(), >> }); >> >> assert!(view.is_node_included("remote-a", "somenode")); >> @@ -512,6 +518,7 @@ fn can_skip_remote_if_excluded() { >> id: "abc".into(), >> include: vec![], >> exclude: vec![FilterRule::Remote("remote-b".to_string())], >> + layout: String::new(), >> }); >> >> assert!(!view.can_skip_remote("remote-a")); >> @@ -524,6 +531,7 @@ fn can_skip_remote_if_included() { >> id: "abc".into(), >> include: vec![FilterRule::Remote("remote-b".to_string())], >> exclude: vec![], >> + layout: String::new(), >> }); >> >> assert!(!view.can_skip_remote("remote-b")); >> @@ -539,6 +547,7 @@ fn can_skip_remote_cannot_skip_if_any_other_include() { >> FilterRule::ResourceId("resource/remote-a/guest/100".to_string()), >> ], >> exclude: vec![], >> + layout: String::new(), >> }); >> >> assert!(!view.can_skip_remote("remote-b")); >> @@ -553,6 +562,7 @@ fn can_skip_remote_explicit_remote_exclude() { >> "resource/remote-a/guest/100".to_string(), >> )], >> exclude: vec![FilterRule::Remote("remote-a".to_string())], >> + layout: String::new(), >> }); >> >> assert!(view.can_skip_remote("remote-a")); >> @@ -564,6 +574,7 @@ fn can_skip_remote_with_empty_config() { >> id: "abc".into(), >> include: vec![], >> exclude: vec![], >> + layout: String::new(), >> }); >> >> assert!(!view.can_skip_remote("remote-a")); >> @@ -578,6 +589,7 @@ fn can_skip_remote_with_no_remote_includes() { >> "resource/remote-a/guest/100".to_string(), >> )], >> exclude: vec![], >> + layout: String::new(), >> }); >> >> assert!(!view.can_skip_remote("remote-a")); >> @@ -590,6 +602,7 @@ fn explicitly_included_remote() { >> id: "abc".into(), >> include: vec![FilterRule::Remote("remote-b".to_string())], >> exclude: vec![], >> + layout: String::new(), >> }); >> >> assert!(view.is_remote_explicitly_included("remote-b")); >> @@ -601,6 +614,7 @@ fn included_and_excluded_same_remote() { >> id: "abc".into(), >> include: vec![FilterRule::Remote("remote-b".to_string())], >> exclude: vec![FilterRule::Remote("remote-b".to_string())], >> + layout: String::new(), >> }); >> >> assert!(!view.is_remote_explicitly_included("remote-b")); >> @@ -612,6 +626,7 @@ fn not_explicitly_included_remote() { >> id: "abc".into(), >> include: vec![], >> exclude: vec![], >> + layout: String::new(), >> }); >> >> // Assert that is not *explicitly* included > _______________________________________________ pdm-devel mailing list pdm-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel