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

clarification for allow_aggregation control #162

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

osquitun
Copy link

@osquitun osquitun commented Mar 8, 2022

clarification for allow_aggregation control

osquitun added 2 commits March 8, 2022 10:42
Clarification on control associated with allow_aggregation
…w-aggregation

Update gnmi-specification.md
configuration of the gNMI service. Additionally, a target SHOULD provide means
sensitive to the latency introduced by bundling, the target MUST follow the
`SubscriptionList` fields in [Section 3.5.1.2](#3512-the-subscriptionlist-message)
that indicate if `allow_aggregation` is permitted.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that maybe we need a larger change to the wording here. I think we should clarify what bundling means here vs. aggregation. Specifically, we were trying to give guidance here as to how many update messages there were per notification.

In aggregation, I think we are looking at how many leaves per update there are. i.e., without allow_aggregation this should be 1.

I'd propose we make this clarification, and then add some text as to when aggregation is triggered. WDYT?

@gwizdms
Copy link
Contributor

gwizdms commented Jun 21, 2022

When allow aggregation is set in the SubscriptionList how does the model (schema) know where to aggregate? An explanation on how it is referenced in the models would help in the specification.

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.

3 participants