ygot icon indicating copy to clipboard operation
ygot copied to clipboard

What to key `ParsedDirectory` with?

Open wenovus opened this issue 3 years ago • 1 comments

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.

wenovus avatar Jul 08 '22 00:07 wenovus

we agreed that:

  • we should key ParsedDirectory on a fully-module qualified path - i.e., module-a:container/module-b:container since this makes the IR robust to any scenario (augment etc.) that can have two nodes with the same name.
  • we discussed that we would implement a function that takes a yang.Entry and returns this new module-qualified path string.
  • this fn can then be used anywhere we index the ParsedDirectory map.

robshakir avatar Aug 05 '22 18:08 robshakir