Re: [pve-devel] [PATCH proxmox-ve-rs v4 15/22] config: sdn: fabrics: add api types

[Gabriel Goller <g.goller@proxmox.com>]

On 08.07.2025 10:15, Wolfgang Bumiller wrote:
>minor doc nits
>
>On Wed, Jul 02, 2025 at 04:50:06PM +0200, Gabriel Goller wrote:
>> [snip]
>> +/// API types for SDN fabric node configurations.
>> +///
>> +/// This module provides specialized types that are used for API interactions when retrieving,
>> +/// creating, or updating fabric/node configurations. These types serialize differently than their
>> +/// section-config configuration counterparts to be nicer client-side.
>> +///
>> +/// The module includes:
>> +/// - [NodeData<T>]: API-friendly version of [NodeSection<T>] that flattens the node identifier
>> +///   into separate `fabric_id` and `node_id` fields
>> +/// - [Node]: API-version of [super::Node]
>> +/// - [NodeDataUpdater]
>> +/// - [NodeDeletableProperties]
>
>^ The types in those links should also be in backticks.

Yep, there is also a `api::` prefix missing on most of these.

>> +///
>> +/// These types include conversion methods to transform between API representations and internal
>> +/// configuration objects.
>> +pub mod api {
>> +    use serde::{Deserialize, Serialize};
>> +
>> +    use proxmox_schema::{Updater, UpdaterType};
>> +
>> +    use crate::sdn::fabric::section_config::protocol::{
>> +        openfabric::{
>> +            OpenfabricNodeDeletableProperties, OpenfabricNodeProperties,
>> +            OpenfabricNodePropertiesUpdater,
>> +        },
>> +        ospf::{OspfNodeDeletableProperties, OspfNodeProperties, OspfNodePropertiesUpdater},
>> +    };
>> +
>> +    use super::*;
>> +
>> +    /// API-equivalent to [NodeSection<T>].
>
>^ backticks

Yep.

>> [snip]
>> +    impl<T> From<NodeData<T>> for NodeSection<T> {
>> +        fn from(value: NodeData<T>) -> Self {
>> +            let id = NodeSectionId::new(value.fabric_id, value.node_id);
>> +
>> +            Self {
>> +                id,
>> +                ip: value.ip,
>> +                ip6: value.ip6,
>> +                properties: value.properties,
>> +            }
>> +        }
>> +    }
>> +
>> +    /// API-equivalent to [super::Node].
>
>^ backticks

Yes.

>> [snip]
>> +    #[derive(Debug, Clone, Serialize, Deserialize)]
>> +    pub struct NodeDataUpdater<T, D> {
>> +        #[serde(skip_serializing_if = "Option::is_none")]
>> +        pub(crate) ip: Option<Ipv4Addr>,
>> +
>> +        #[serde(skip_serializing_if = "Option::is_none")]
>> +        pub(crate) ip6: Option<Ipv6Addr>,
>> +
>> +        #[serde(flatten)]
>> +        pub(crate) properties: T,
>> +
>> +        #[serde(skip_serializing_if = "Vec::is_empty", default = "Vec::new")]
>
>(^ the `= "Vec::new"` should not be necessary, but doesn't matter
>either - new is `const` so it's probably more efficient this way
>anyawy...)

Yeah, Vec::default calls Vec::new anyway. I'll still keep it like this
though.

>> +        pub(crate) delete: Vec<NodeDeletableProperties<D>>,
>> +    }
>> +
>> [snip]

Thanks!


_______________________________________________
pve-devel mailing list
pve-devel@lists.proxmox.com
https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel