Re: [pve-devel] [PATCH proxmox-perl-rs v4 2/5] pve-rs: sdn: fabrics: add api methods

[Wolfgang Bumiller <w.bumiller@proxmox.com>]

On Fri, Jul 04, 2025 at 05:56:46PM +0200, Gabriel Goller wrote:
> On 04.07.2025 14:57, Wolfgang Bumiller wrote:
> > On Wed, Jul 02, 2025 at 04:50:15PM +0200, Gabriel Goller wrote:
> > > From: Stefan Hanreich <s.hanreich@proxmox.com>
> > > 
> > > The FabricConfig from proxmox-ve-config implements CRUD functionality
> > > for Fabrics and Nodes stored in the section config. We expose them via
> > > perlmod, so they can be used in the API endpoints defined in perl.
> > > they map 1:1 to the respective API endpoints.
> > > 
> > > They are simply calling the respective implementation of FabricConfig,
> > > and convert from / to the API representations of the Fabrics / Nodes
> > > returned by FabricConfig.
> > > 
> > > Co-authored-by: Gabriel Goller <g.goller@proxmox.com>
> > > Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com>
> > > ---
> > >  pve-rs/src/bindings/sdn/fabrics.rs | 212 ++++++++++++++++++++++++++++-
> > >  1 file changed, 211 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/pve-rs/src/bindings/sdn/fabrics.rs b/pve-rs/src/bindings/sdn/fabrics.rs
> > > index fac5602c0241..2efa1c6306ae 100644
> > > --- a/pve-rs/src/bindings/sdn/fabrics.rs
> > > +++ b/pve-rs/src/bindings/sdn/fabrics.rs
> > > @@ -17,7 +17,20 @@ pub mod pve_rs_sdn_fabrics {
> > >      use proxmox_section_config::typed::SectionConfigData;
> > >      use proxmox_ve_config::common::valid::Validatable;
> > > 
> > > -    use proxmox_ve_config::sdn::fabric::{section_config::Section, FabricConfig};
> > > +    use proxmox_ve_config::sdn::fabric::{
> > > +        section_config::{
> > > +            fabric::{
> > > +                api::{Fabric, FabricUpdater},
> > > +                FabricId,
> > > +            },
> > > +            node::{
> > > +                api::{Node, NodeUpdater},
> > > +                Node as ConfigNode, NodeId,
> > > +            },
> > > +            Section,
> > > +        },
> > > +        FabricConfig, FabricEntry,
> > > +    };
> > > 
> > >      /// A SDN Fabric config instance.
> > >      #[derive(Serialize, Deserialize)]
> > > @@ -57,6 +70,203 @@ pub mod pve_rs_sdn_fabrics {
> > >          )
> > >      }
> > > 
> > > +    /// Class method: Returns all fabrics and nodes from the configuration.
> > 
> > ^ Regular `Method:` - all of them.
> 
> Done.
> 
> > > +    /// Class method: Adds a new Fabric to the configuration.
> > > +    #[export]
> > > +    fn add_fabric(#[try_from_ref] this: &PerlFabricConfig, fabric: Fabric) -> Result<(), Error> {
> > > +        this.fabric_config
> > > +            .lock()
> > > +            .unwrap()
> > > +            .add_fabric(fabric)
> > 
> > ^ If all we do is forward to an existing method, it would be nice to
> > include a `See [...]` link in the method docs.
> > (The `make doc/doc-open` make targets currently add
> > `--external-html-root-url` parameters to `cargo doc` to make these links
> > work (just updated to the new rustdoc layout).
> 
> Added it, even though the function won't be visible in the docs because
> it isn't pub.

Ah yes, I forgot to check for that.

It *should* be pub ;-)

The function is exposed to perl, which means it *is* public (just not
within rust) - the perl API is *supposed* to be documented. That's kind
of the point of the `bindings` module, to have rustdoc generated an API
documentation for perl.

This is probably something perlmod should optionally check for: non-pub
exports should need some kind of extra annotation.


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