Skip to content

Update style guide to reflect path compression rules.#1424

Open
robshakir wants to merge 5 commits intomasterfrom
robjs/style-guide
Open

Update style guide to reflect path compression rules.#1424
robshakir wants to merge 5 commits intomasterfrom
robjs/style-guide

Conversation

@robshakir
Copy link
Copy Markdown
Member

 * (M) doc/openconfig_style_guide.md
   - Provide further documentation in the OpenConfig style guide
     on the rules required to allow programmatic schema transformation.
     These rules have been adhered to throughout the lifetime of
     OpenConfig, and are implemented by widely used code generation
     tools. This PR simply records these defacto style guide rules.

     This PR is accompanied by https://github.com/openconfig/oc-pyang/pull/59
     which implements the relevant linter checks for the rules
     described herein.

 * (M) doc/openconfig_style_guide.md
   - Provide further documentation in the OpenConfig style guide
     on the rules required to allow programmatic schema transformation.
     These rules have been adhered to throughout the lifetime of
     OpenConfig, and are implemented by widely used code generation
     tools. This PR simply records these defacto style guide rules.

     This PR is accompanied by openconfig/oc-pyang#59
     which implements the relevant linter checks for the rules
     described herein.
@OpenConfigBot
Copy link
Copy Markdown

OpenConfigBot commented Jan 10, 2026

No major YANG version changes in commit 84056b2

@earies
Copy link
Copy Markdown
Contributor

earies commented Jan 15, 2026

Just wanted to post an overall comment/observation - this style proposal is to accomodate patterns for collapsing schema nodes and codegen for ygot and just want to confirm that that then means ygot would not be able to accomodate patterns that fall outside of this boundary - is that correct? (e.g. other model-sets)

I'll admit I haven't looked across various repos verbiage to this matter but the interoperability between OC projects and content outside of OC projects scope has been a growing concern that I'm not sure has been fully articulated other than YMMV.

If that is all correct, do we agree the following from ygot README is sufficient to articulate this?

Whilst ygot is designed to work with any YANG module, for OpenConfig modules, it can provide transformations of
the schema to optimise the data structures that are produced for use in systems that generate data instances of the
models for configuration purposes. These helper methods require that the OpenConfig style guide patterns are implemented, a model can be verified to conform with these requirements using the OpenConfig linter.

@robshakir
Copy link
Copy Markdown
Member Author

Just wanted to post an overall comment/observation - this style proposal is to accomodate patterns for collapsing schema nodes and codegen for ygot and just want to confirm that that then means ygot would not be able to accomodate patterns that fall outside of this boundary - is that correct? (e.g. other model-sets)

ygot has never done schema compression (what it refers to these transformations as) for any other type of model. It could be extended to do so, but it hasn't been thus far.

I'll admit I haven't looked across various repos verbiage to this matter but the interoperability between OC projects and content outside of OC projects scope has been a growing concern that I'm not sure has been fully articulated other than YMMV.

I'm not sure I understand this comment. Is your concern that ygot is outside of the OpenConfig project's scope?

If that is all correct, do we agree the following from ygot README is sufficient to articulate this?

Whilst ygot is designed to work with any YANG module, for OpenConfig modules, it can provide transformations of the schema to optimise the data structures that are produced for use in systems that generate data instances of the models for configuration purposes. These helper methods require that the OpenConfig style guide patterns are implemented, a model can be verified to conform with these requirements using the OpenConfig linter.

@earies
Copy link
Copy Markdown
Contributor

earies commented Jan 15, 2026

ygot has never done schema compression (what it refers to these transformations as) for any other type of model. It could be extended to do so, but it hasn't been thus far.

I haven't dug into ygot sources but are you suggesting the schema compression and schema design assumptions are specific to OC model-sets (schema aware)? And if used against alternate model-sets where there is likelyhood to breach these rulesets, ygot can support (assuming coverage of the entirety of YANG 1.0/1.1 language) but not using these compression rules?

I'll admit I haven't looked across various repos verbiage to this matter but the interoperability between OC projects and content outside of OC projects scope has been a growing concern that I'm not sure has been fully articulated other than YMMV.

I'm not sure I understand this comment. Is your concern that ygot is outside of the OpenConfig project's scope?

Was just trying to understand since this imposes a design protocol on the YANG modeling that if this means ygot and OC YANG modeling are only interoperable as a result and how ygot might handle schemas that cannot conform to this ruleset (e.g. native, IETF, etc...)

@dplore
Copy link
Copy Markdown
Member

dplore commented Jan 21, 2026

ygot has never done schema compression (what it refers to these transformations as) for any other type of model. It could be extended to do so, but it hasn't been thus far.

I haven't dug into ygot sources but are you suggesting the schema compression and schema design assumptions are specific to OC model-sets (schema aware)? And if used against alternate model-sets where there is likelyhood to breach these rulesets, ygot can support (assuming coverage of the entirety of YANG 1.0/1.1 language) but not using these compression rules?

I'll admit I haven't looked across various repos verbiage to this matter but the interoperability between OC projects and content outside of OC projects scope has been a growing concern that I'm not sure has been fully articulated other than YMMV.

I'm not sure I understand this comment. Is your concern that ygot is outside of the OpenConfig project's scope?

Was just trying to understand since this imposes a design protocol on the YANG modeling that if this means ygot and OC YANG modeling are only interoperable as a result and how ygot might handle schemas that cannot conform to this ruleset (e.g. native, IETF, etc...)

Compression is an optional feature of ygot. It can be turned off. ygot has accepted changes to help it consume non-OC models. (ie: openconfig/ygot#1030) But we want to formalize with this PR that we choose a style for OC models which are compressible.

@robshakir
Copy link
Copy Markdown
Member Author

ygot has never done schema compression (what it refers to these transformations as) for any other type of model. It could be extended to do so, but it hasn't been thus far.

I haven't dug into ygot sources but are you suggesting the schema compression and schema design assumptions are specific to OC model-sets (schema aware)? And if used against alternate model-sets where there is likelyhood to breach these rulesets, ygot can support (assuming coverage of the entirety of YANG 1.0/1.1 language) but not using these compression rules?

That's absolutely correct. There are active users of goyang and ygot that are using models other than OpenConfig. The compression rules are specifically to improve developer experience for models that comply with OpenConfig's style rules. Today, this is actively used for both OpenConfig and OpenTrafficGenerator models.

I'll admit I haven't looked across various repos verbiage to this matter but the interoperability between OC projects and content outside of OC projects scope has been a growing concern that I'm not sure has been fully articulated other than YMMV.

I'm not sure I understand this comment. Is your concern that ygot is outside of the OpenConfig project's scope?

Was just trying to understand since this imposes a design protocol on the YANG modeling that if this means ygot and OC YANG modeling are only interoperable as a result and how ygot might handle schemas that cannot conform to this ruleset (e.g. native, IETF, etc...)

This change simply makes a rule that has been adhered to throughout the modelling explicit (i.e., converts it from a convention to a "rule"). ygot will always support schemas using the "uncompressed" mode (and has done for ~10 years in production network deployments :-)). Essentially, ygot is a generic toolkit for YANG users in Go, but has a "works with OpenConfig" badge since a number of its primary users use OpenConfig. :-)

@robshakir
Copy link
Copy Markdown
Member Author

@ElodinLaarz @jsterne Thanks for the review. PTAL at the edits.

@jsterne
Copy link
Copy Markdown

jsterne commented Feb 3, 2026

Looks much better now - thx. I wasn't sure if the rule extends beyond the grandparent (hence why I was mentioning "ancestor" in my comments). So I take it the following combination is allowed?

/foo/bar/stuff/even-more/a
/foo/a

(i.e. the compression is really only for immediately adjacent segments of a path?)

@dplore dplore moved this to Ready to discuss in OC Operator Review Feb 23, 2026
@dplore
Copy link
Copy Markdown
Member

dplore commented Feb 23, 2026

@ElodinLaarz can you give another review for us?

Copy link
Copy Markdown
Contributor

@ElodinLaarz ElodinLaarz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some things to tidy up, but generally LGTM.

`/a/foos/foo` and `/interfaces/interface` are `list` nodes. This rule
exists to allow the (style-guide-required) "surrounding" container of
a list to be removed during schema transformation.
* **A leaf node may not share a name with a grandparent**. It is not legal
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

**A leaf node may not share a name with a grandparent**.

Do we want this here still? Seems unrelated to the explanation that follows. (And is also not a true statement -- presumably having been replaced with the previous bullet about children of its grandparent.)

Comment on lines +503 to +505
and `/b/leaf` to both exist. The single exception to this rule is the
special case of nodes which act as the `key` of a `list`, where the leaf
must be a direct child of the `list` node. Such leaves MUST be the `key`
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Optional way to rephrase (also fine as is):

"The single exception to this rule is the OpenConfig list key pattern. The leaf inside the config or state container is permitted to share its name with the leafref node acting as the key of the parent list."

`/a/foos/foo` and `/interfaces/interface` are `list` nodes. This rule
exists to allow the (style-guide-required) "surrounding" container of
a list to be removed during schema transformation.
* **A leaf node may not share a name with a grandparent**. It is not legal
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, if you keep some requirement around you probably want to use must vs may, per RFC 2119 -- MAY (optional) vs MUST (required).

@ElodinLaarz ElodinLaarz moved this from Ready to discuss to last-call in OC Operator Review Feb 24, 2026
@ElodinLaarz
Copy link
Copy Markdown
Contributor

Discussed at OC Operators Meeting on Feb 24, 2026:

Assuming the couple of changes noted are resolved, we can merge in 2 weeks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: last-call

Development

Successfully merging this pull request may close these issues.

6 participants