Re: [pve-devel] [PATCH proxmox-ve-rs 03/11] add intermediate fabric representation

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

>> [snip]
>> +#[derive(Serialize, Deserialize, Debug, Default)]
>> +pub struct FabricConfig {
>> +    openfabric: Option<openfabric::internal::OpenFabricConfig>,
>> +    ospf: Option<ospf::internal::OspfConfig>,
>> +}
>> +
>> +impl FabricConfig {
>> +    pub fn new(raw_openfabric: &str, raw_ospf: &str) -> Result<Self, anyhow::Error> {
>> +        let openfabric =
>> +            openfabric::internal::OpenFabricConfig::default(raw_openfabric)?;
>> +        let ospf = ospf::internal::OspfConfig::default(raw_ospf)?;
>
>Maybe rename the two methods to new, since default usually has no
>arguments and this kinda breaks with this convention?

Good point.

>> +#[derive(Error, Debug)]
>> +pub enum IntegerRangeError {
>> +    #[error("The value must be between {min} and {max} seconds")]
>> +    OutOfRange { min: i32, max: i32 },
>> +    #[error("Error parsing to number")]
>> +    ParsingError(#[from] ParseIntError),
>> +}
>> +
>> +#[derive(Serialize, Deserialize, Hash, Debug, Clone, PartialEq, Eq, PartialOrd, Ord)]
>
>derive Copy for ergonomics

Done – also for all the other types.

>> +    impl TryFrom<InterfaceProperties> for Interface {
>> +        type Error = OpenFabricConfigError;
>> +
>> +        fn try_from(value: InterfaceProperties) -> Result<Self, Self::Error> {
>> +            Ok(Interface {
>> +                name: value.name.clone(),
>> +                passive: value.passive(),
>> +                hello_interval: value.hello_interval,
>> +                csnp_interval: value.csnp_interval,
>> +                hello_multiplier: value.hello_multiplier,
>> +            })
>> +        }
>> +    }
>
>are we anticipating this to be fallible in the future?

Hmm not really. These simple properties all have the same type in the
SectionConfig so they are validated already. Same goes for a few other
SectionConfig -> IntermediateConfig conversions.
Changed them to simple From impls.

Note that a few (e.g. Node) conversions need to be TryFrom as we need to
parse the router_id as an IpV4Addr.

Thanks for the review!


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