[snip] + impl PerlSectionConfig<OpenFabricSectionConfig> { + pub fn add_fabric(&self, new_config: AddFabric) -> Result<(), anyhow::Error> { + let fabricid = FabricId::from(new_config.name).to_string();Could we simplify this method and the ones below by just using the concrete types (here FabricId) inside the argument structs (AddFabric)? There's potential for quite a few here afaict, also with the Option<u16>'s. Would save us a lot of conversion / validation logic if we just did it at deserialization.
Yep, that would work. We just need to change the serde deserialize override to be generic.
I pointed out some instances below. I guess the error messages would be a bit worse then?
Nope, they are quite the same. We can just wrap them in serde custom errors.
+ let new_fabric = OpenFabricSectionConfig::Fabric(FabricSection { + hello_interval: new_config + .hello_interval + .map(|x| x.try_into()) + .transpose()?, + }); + let mut config = self.section_config.lock().unwrap(); + if config.sections.contains_key(&fabricid) { + anyhow::bail!("fabric already exists"); + } + config.sections.insert(fabricid, new_fabric);try_insert instead of contains_key + insert?
still deprecated :)
[snip] + let mut config = self.section_config.lock().unwrap(); + if !config.sections.contains_key(&nodeid) { + anyhow::bail!("node not found"); + } + config.sections.entry(nodeid).and_modify(|n| { + if let OpenFabricSectionConfig::Node(n) = n { + n.net = net; + n.interface = interfaces; + } + });wouldn't get_mut be easier here? also would save the extra contains_key
Agree, also changed everywhere else. Thanks! _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
