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

Update gnsi-acctz.yang #145

Closed
wants to merge 2 commits into from
Closed

Update gnsi-acctz.yang #145

wants to merge 2 commits into from

Conversation

marcushines
Copy link
Contributor

Remove duplicate container for counters

Remove duplicate container for counters
acctz/gnsi-acctz.yang Show resolved Hide resolved
(January 1st, 1970 00:00:00 GMT).";
}
"A collection of counters from the gNSI.acctz module. This collection
augments the existing certz grpc counters";

container client-counters {
Copy link
Contributor

Choose a reason for hiding this comment

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

un-indent these children of the counters container?

for acctz clients and sources.";
config false;

leaf last-cleared-on {
Copy link
Contributor

Choose a reason for hiding this comment

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

It's ok for this leaf to no longer exist anywhere?

@@ -25,6 +25,12 @@ module gnsi-acctz {
"This module provides a data model for the metadata of the gRPC
accounting operations on a device.";

revision 2023-01-24 {
description
"Remove duplicate counter container.";
Copy link
Contributor

Choose a reason for hiding this comment

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

"counter" -> "counters"

@dplore
Copy link
Member

dplore commented Jan 24, 2024

While you are here making these changes we should include a state container in the path. OC style guide, requires a state container for paths representing operational state.

The most common,(but not style guide required grouping) for counters is to use a state/counters container.

  • /system/grpc-servers/grpc-server/state/counters/client-records

Less common, but has precedence in igmp and isis models and is closer to what you have here would be one of the following:

  • /system/grpc-servers/grpc-server/client-counters/state/records
  • /system/grpc-servers/grpc-server/state/client-counters/records

@wenovus
Copy link

wenovus commented Jan 26, 2024

We're looking at resolving these issues at openconfig/public#1037

Propose to close this PR and move the discussion over there.

@marcushines marcushines closed this Feb 1, 2024
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