ygot icon indicating copy to clipboard operation
ygot copied to clipboard

when creating a keyed list entry, the reference node of the key should be created first

Open larryleguo opened this issue 6 years ago • 4 comments

In the generated go struct code:

func (t *OpenconfigPlatform_Components) NewComponent(Name string) (*OpenconfigPlatform_Components_Component, error)

creates a OpenconfigPlatform_Components_Component object with the specified name. However, name is a ref leaf, so the underlying config node should also be created.

The object created by NewComponent() fails on validation.

larryleguo avatar Mar 01 '18 19:03 larryleguo

New... methods indeed do not have something that covers this use case if CompressOCPaths is not enabled. It would be possible to implement this relatively trivially, particularly by:

  • Checking during New... generation whether the key leaf is of type leafref.
  • Checking the path of the key leafref, and if it resolves to a leaf under the current list member, creating the relevant hierarchy within the generated object.

I'm happy to review CLs that add to the code for this if you have an urgent need for it.

robshakir avatar Mar 02 '18 23:03 robshakir

This seems to be implemented today whether or not compressPaths is true.

From unexampleoc/oc.go

func (t *OpenconfigPlatform_Components) NewComponent(Name string) (*OpenconfigPlatform_Components_Component, error) {

        // Initialise the list within the receiver struct if it has not already been
        // created.
        if t.Component == nil {
                t.Component = make(map[string]*OpenconfigPlatform_Components_Component)
        }

        key := Name

        // Ensure that this key has not already been used in the
        // list. Keyed YANG lists do not allow duplicate keys to
        // be created.
        if _, ok := t.Component[key]; ok {
                return nil, fmt.Errorf("duplicate key %v for list Component", key)
        }

        t.Component[key] = &OpenconfigPlatform_Components_Component{
                Name: &Name,
        }

        return t.Component[key], nil
}

wenovus avatar Jun 01 '20 22:06 wenovus

Wen,

There's a slight difference here -- we expect that creating a new entry eth0 under /interfaces/interface, then we create (in the YANG schema):

  • /interfaces/interface/name = eth0
  • /interfaces/interface/config/name = eth0

In the ygot mapped schema, we create the key eth0 in the map for the Interface list.

For compressed cases /interfaces/interface/name and /interfaces/interface/config/name are the same Go field (since we map to two paths), but in the uncompressed cases there are two paths to populate. Particularly:

  • We have struct OpenconfigInterfaces_Interfaces_Interface with a field Name that should be populated (it is, as your paste shows).
  • We have struct OpenconfigInterfaces_Interfaces_Inteface_Config with field Name that should be populated -- this one isn't.

The original report here is that if one were to do just fakeroot.Interfaces.NewInterface("eth0") then fakeroot.Validate() will fail because the leaf /interfaces/interface/name is a leafref to ../config/name, which is unpopulated.

The change which I suggested above is essentially to create the Interfaces_Interface_Config struct with Name populated, and assign it to the Config field of the NewInterface that was created. This requires some examination in the code generation to know that the list key was a leafref to a child of the list itself, and to generate that code.

This is a pretty old feature request, happy to close it, but we should just note for the record that we don't implement this today :-)

Cheers, r.

robshakir avatar Jun 01 '20 23:06 robshakir

Ahh oops, my mistake not noticing the config node underneath. I'm happy to keep this open as a low priority item (since there has not been discussion since).

wenovus avatar Jun 01 '20 23:06 wenovus