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

User-defined defaults and description updates for list keys #599

Merged
merged 3 commits into from
Apr 20, 2022

Conversation

earies
Copy link
Contributor

@earies earies commented Mar 8, 2022

  • (M) release/models/network-instance/openconfig-network-instance-l2.yang
  • (M) release/models/network-instance/openconfig-network-instance.yang
    • Update network-instance name description to provide implementation
      guidance on default name for DEFAULT_INSTANCE should OpenConfig
      schemas not be used for configuration but rather for state only as
      well as suggested name
    • Update protocol instance name with suggested value as well as
      default name should OpenConfig schemas not be used for
      configuration but rather for state only
  • (M) release/models/system/openconfig-system-grpc.yang
    • Update grpc server name with suggested value as well as default
      name should OpenConfig schemas not be used for configuration but
      rather for state only

This PR is to kick off the concept of embedding implementation suggestions (and
defaults where applicable) for where user-defined fields are used as list keys.
Most notably starting in areas where users have a hard time selecting an
appropriate value and where an implementation may only support single instances
in a multi-instance capable model structure.

The notion of implementation guidance and default values also caters to
scenarios where a client may want to consume OpenConfig modeled state data
without using OpenConfig modeled configuration. These suggestions and defaults
enable the implementation to pack known values into list keys to formulate
compliant paths.

@earies earies force-pushed the user-defined-defaults branch from f238a71 to 53057b7 Compare March 8, 2022 01:13
@OpenConfigBot
Copy link

OpenConfigBot commented Mar 8, 2022

Compatibility Report for commit 5154323:
yanglint@SO 1.10.17

@earies earies force-pushed the user-defined-defaults branch 2 times, most recently from 3f6cd68 to f430de9 Compare March 8, 2022 01:29
  * (M) release/models/network-instance/openconfig-network-instance-l2.yang
  * (M) release/models/network-instance/openconfig-network-instance.yang
    - Update network-instance name description to provide implementation
      guidance on default name for DEFAULT_INSTANCE should OpenConfig
      schemas not be used for configuration but rather for state only as
      well as suggested name
    - Update protocol instance name with suggested value as well as
      default name should OpenConfig schemas not be used for
      configuration but rather for state only
  * (M) release/models/system/openconfig-system-grpc.yang
    - Update grpc server name with suggested value as well as default
      name should OpenConfig schemas not be used for configuration but
      rather for state only
@earies earies force-pushed the user-defined-defaults branch from f430de9 to ab4173b Compare March 8, 2022 01:32
Copy link
Contributor

@robshakir robshakir left a comment

Choose a reason for hiding this comment

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

Thanks for suggesting this Ebben, I definitely think its worth discussing.

One area that I think we probably should discuss is the relationship between some of these defaults and the config used at boot time. There is an implication in some of the comments that some of these values are mutable post-initialisation, which I think has some implementation implications that we might want to consider.

I'm fine, in general, though with an approach that says "an implementor is recommended to use this name as a default value".

@dplore dplore requested a review from robshakir April 14, 2022 16:42
@dplore
Copy link
Member

dplore commented Apr 19, 2022

This was discussed with the operators group today. Feedback was we should make this a major revision and point out in this PR that this is may be a backwards incompatible change for some implementations due to what could be a NEW default value of a network instance name (for example if a vendor currently has some default name string such as "BASE" and this change requires “DEFAULT”

@earies
Copy link
Contributor Author

earies commented Apr 19, 2022

Thx @dplore - sounds reasonable to me. Will update the version and revision descriptions to reflect this across the models

@dplore dplore merged commit bfbe682 into openconfig:master Apr 20, 2022
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

Successfully merging this pull request may close these issues.

4 participants