openconfig / ygot

A YANG-centric Go toolkit - Go/Protobuf Code Generation; Validation; Marshaling/Unmarshaling
Apache License 2.0
286 stars 107 forks source link

What to key `ParsedDirectory` with? #710

Open wenovus opened 2 years ago

wenovus commented 2 years ago

Currently, ParsedDirectory is keyed on the "total" schema path of the node. So this includes choice/cases statements for a YANG schema that uses them. This was noted as questionable by https://github.com/openconfig/ygot/pull/655#discussion_r859288696, where I assume @robshakir means we don't need to have choice/case statements in the path because we always only choose a single child during code generation, and therefore we can remove this form of path to minimize the complexity we currently have with Path vs. SchemaPath as you mentioned https://github.com/openconfig/ygot/pull/655#discussion_r859288696.

Copying the code below for convenience: https://github.com/openconfig/ygot/blob/e7b18d306e0883a150758dd89d965a33bd57e49b/ygen/ir.go#L647-L670

@robshakir Could you check whether my understanding of the motivation for this issue is correct?

There are two alternatives that come to mind: 1) key on the path without choice/case statements, and 2) key on the path without choice/case statements, but also qualify each element with the namespace of the node. In 2) the type of the path can be a custom type YANGPath string in order to provide methods for getting the unqualified path and just the first element (which is the module name). The former is simpler, but the latter provides more information and also takes away the RootModule field in the IR as you mentioned in https://github.com/openconfig/ygot/pull/657#discussion_r859294556. I'm leaning towards 2).

A cursory look at the code dependence suggests to me that it will be doable to make the change, since I'm not seeing any dependence on the choice/case elements.

robshakir commented 2 years ago

we agreed that: