Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OpenConfig Compressed Names for Uncompressed GoStructs #755

Open
wenovus opened this issue Dec 21, 2022 · 5 comments
Open

OpenConfig Compressed Names for Uncompressed GoStructs #755

wenovus opened this issue Dec 21, 2022 · 5 comments

Comments

@wenovus
Copy link
Collaborator

wenovus commented Dec 21, 2022

Background

Compressed GoStructs naming is much more terse than uncompressed GoStructs:
e.g. Lldp_Interface_Neighbor vs. OpenconfigLldp_Lldp_Interfaces_Interface_Neighbors_Neighbor
e.g. exampleoc.Interface_OperStatus_UP vs. uexampleoc.OpenconfigInterfaces_Interfaces_Interface_State_OperStatus_UP

However, some hard constraints prevent the use of compressed GoStructs:

  • Augments to OpenConfig models that break the OpenConfig style guide.
  • Need for Config/State separation in the generated GoStructs.

In particular it can be very expensive to make modifications to existing YANG models, such that whenever a modified version of OpenConfig models are being used, one is forced to use uncompressed GoStructs' verbose names.

Therefore, there exists an opportunity to remove OpenConfig naming redundancies (i.e. module and surrounding container names) for uncompressed GoStructs whenever uncompressed GoStructs are desired.

Requirements

This document proposes to shorten struct and enum/identityref names in such a way that,

  1. Names match compressed GoStructs as much as possible.
  2. Names maintain uniqueness in spite of extra nodes (e.g. list surrounding containers, config/state) while maintaining readability.
  3. This change must purely be a naming optimization, and MUST not put requirements on the structure of the input YANG.

Struct names

These are shortened in the same way as compressed structs, with the following exceptions:

  1. Config/state containers are not omitted from the name. This allows these to deviate from one another.
  2. List surrounding containers are not omitted from the name IF it is the last element (i.e. we're generating the container type). Furthermore, if the name matches that of the list, then _Container is appended as its suffix.
  3. List surrounding containers are not omitted from the name if it has more than one child.

Enum names

These are shortened in the same way as compressed structs, with the following exception:

  1. Config/state versions of enums/identityrefs are not de-duped. This allows two different enums to exist with the same name in between config/state.
@robshakir
Copy link
Contributor

robshakir commented Dec 21, 2022

To be clear, this proposal is for OpenConfig models specifically? i.e., we will add a new flag that says "this is a model that still complies with OpenConfig rules, but we want to have config/state split only"?

This is an important distinction because we cannot tell that these models are meant to be OpenConfig-style compliant, and for a general YANG model the rules that are relied upon cannot be guaranteed.

@wenovus
Copy link
Collaborator Author

wenovus commented Dec 21, 2022

It's mainly intended for pure OpenConfig or non-pure (augmented) OpenConfig models where there is a partial conformance with the OpenConfig-style.

I intend to add a generation flag:

compressOCNames         = flag.Bool("compress_oc_names", false, "If set to true, ygot will attempt to compress names even when compress_paths=false, e.g. list containers will be given a \"_Container\" suffix where the surrounding container shares the same name.")

It's ok that the input YANG is not strictly OpenConfig-style compliant, because when it's not compliant (e.g. surrounding list container has more than one child) we can simply not apply the name-shortening. The advantage, however, is that the rule will apply most of the time in the case of pure OpenConfig or non-pure (augmented) OpenConfig models.

@robshakir
Copy link
Contributor

I think it is dangerous to try and do this for models that are not compliant with a particular style, there are many things that can change about those models that make things backwards incompatible. I'd prefer that this is explicitly OpenConfig, and we say that the augmented model set MUST pass the linter to have clear expectations.

Take some schema that today has /interfaces/interface - if someone comes along and does an augmentation of /interfaces/other-interfaces, you are going to end up with a change of type name for the first container which will break existing code. OpenConfig's linter will prevent this augmentation being considered valid, but general YANG cannot make such guarantees.

@wenovus
Copy link
Collaborator Author

wenovus commented Dec 21, 2022

Ok, so we're explicitly saying that ygot will maintain generated-code backward-compatibility if the YANG is also changed in a backward-compatible way? I see field additions as the main example, so uses, augment, or simply a new leaf/subtree.

I agree this can be annoying since there can be a lot of changes involved for a single augment.

The problem I'm trying to solve is where the YANG is not pure OpenConfig that strictly obeys the style guide, but where users would still like to take advantage of shortened names. Maybe with autocomplete this is not as big of a problem as this name-changing issue.

@wenovus
Copy link
Collaborator Author

wenovus commented Dec 21, 2022

@robshakir Your comment on "OpenConfig only" gave me an idea that by segregating the name generation and looking at the originating module namespace of a node (I.e. whether the node is defined in an OpenConfig module), we can avoid the backward compatibility issue by these augments not affecting the names of OpenConfig-originated leafs/subtrees.

This also has the advantage of allowing for conscious "misuse" where the user takes responsibility for non-style-compliant OpenConfig, allowing for the lenient generation for modified/augmented OC YANG. It seems some users would rather deal with the backward-incompatibility than the long names.

The root cause is actually due to the generated code being too large and autocomplete systems taking a long time to warm up. However I'm not yet convinced of aliasing as a solution because it itself may not be something that autocomplete tools generally well supports. I think this is worth an investigation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants