-
Notifications
You must be signed in to change notification settings - Fork 88
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
What should a gNMI server return when a default value is queried? #142
Comments
@dan-lepage pointed out the interesting case where My current preference: By requiring GET to always return a value for a leaf GET/SUBSCRIBE if it exists, even if it's the default value, it removes this ambiguity. This is also relevant for non-leafs. While I'm guessing that list keys should not have default values specified, it's possible to have containers underneath lists that make it possible for the same ambiguity to occur. Although this case may occur less often, we may still need to consider it for completeness. Related question: Are we planning on supporting presence-containers? |
If I'm reading the spec correctly, it doesn't actually say what should happen on a GET. On a SUBSCRIBE with ONCE set, it should actually never return - "In the case that a particular path does not (yet) exist, the target MUST NOT close the RPC, and instead should continue to monitor for the existence of the path, and transmit telemetry updates should it exist in the future." How does a GET return "nothing" for a syntactically valid but nonexistent path? E.g. does GET on |
Yes I mixed them up, I agree with you on both GET and SUBSCRIBE. I'm now interpreting the spec to say that SUBSCRIBE[ONCE] would hang if the path doesn't exist. |
Subscribe[once] should never "hang". It should return only what it has
"now". The wording around not closing the RPC is specific to the streaming
such that a missing path should never be a hard error which enables
watching for something you expect to appear.
…On Thu, Sep 2, 2021 at 4:05 PM Wen Bo Li ***@***.***> wrote:
Yes I mixed them up, I agree with you on both GET and SUBSCRIBE. I'm now
interpreting the spec to say that SUBSCRIBE[ONCE] would hang if the path
doesn't exist.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#142 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AEL4QL47Q7GNQLCDGVRHU5DT77KIHANCNFSM5BDIAKOQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
On the Get, the response should be empty, no updates, not a value
indicating empty.
…On Fri, Sep 10, 2021 at 1:36 PM Carl Lebsack ***@***.***> wrote:
Subscribe[once] should never "hang". It should return only what it has
"now". The wording around not closing the RPC is specific to the streaming
such that a missing path should never be a hard error which enables
watching for something you expect to appear.
On Thu, Sep 2, 2021 at 4:05 PM Wen Bo Li ***@***.***> wrote:
> Yes I mixed them up, I agree with you on both GET and SUBSCRIBE. I'm now
> interpreting the spec to say that SUBSCRIBE[ONCE] would hang if the path
> doesn't exist.
>
> —
> You are receiving this because you are subscribed to this thread.
> Reply to this email directly, view it on GitHub
> <#142 (comment)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AEL4QL47Q7GNQLCDGVRHU5DT77KIHANCNFSM5BDIAKOQ>
> .
> Triage notifications on the go with GitHub Mobile for iOS
> <https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
> or Android
> <https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
>
>
|
Ok so the wording around closing the RPC in section 3.5.1.3 only applies to 3.5.1.5.2 (STREAM), but not 3.5.1.5.1 (ONCE)? If so I think the spec should be edited to make this clear. |
Suggested change to 3.5.1.3 I wrapped added parts with @@ There is no requirement that the path specified in the message must exist within There is no requirement that the path specified in the message must exist within |
This SGTM
…On Fri, Sep 10, 2021 at 3:01 PM Wen Bo Li ***@***.***> wrote:
Suggested change to 3.5.1.3
I wrapped added parts with @@
There is no requirement that the path specified in the message must exist
within
the current data tree on the server. While the path within the subscription
SHOULD be a valid path within the set of schema modules that the target
supports, subscribing to any syntactically valid path within such modules
MUST
be allowed. In the case that a particular path does not (yet) exist, the
target
MUST NOT close the RPC, and instead should continue to monitor for the
existence
of the path, and transmit telemetry updates should it exist in the future.
------------------------------
There is no requirement that the path specified in the message must exist
within
the current data tree on the server. While the path within the subscription
SHOULD be a valid path within the set of schema modules that the target
supports, subscribing to any syntactically valid path within such modules
MUST
be allowed. In the case that a particular path does not (yet) exist, *@@*
the target
would send just a sync_response for ONCE and POLL subscriptions, but
MUST NOT close the RPC for STREAM subscriptions *@@*, and instead should
continue to monitor for the existence
of the path, and transmit telemetry updates should it exist in the future.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#142 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AEL4QL4ANAJKLMRBA6R4QGDUBJIYDANCNFSM5BDIAKOQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
|
@robshakir Do you agree that a device should send the default value to the user as-if it's a leaf without a default, and set to that value? |
If I may pitch in, perhaps config leaves could be allowed either nil or the actual default value, but I feel that the state leaves should always return the effective value. That seems to be consistent with the separation of config and state containers, where admin settings could differ from the operational state. |
This is a relatively complex issue. I suggest we break this up into several smaller issues which we can perhaps solve independently. For this issue let's focus on the SUBSCRIBE behavior. Here's some suggested wording:
I will open additional issues for GET regarding default values and GET for a subtree |
I commented similarly on #155 - the spec right now says that if you REPLACE a node and don't specify a value for a child, that child should be set to its default value. So I would argue that this isn't two separate issues for subscribe and get, but rather an issue with REPLACE, DELETE, and the initial value of the OpenConfig tree: if we have DELETE behave like an empty REPLACE, then it should be impossible for config or state values that have defaults to ever be unset, because any attempt to erase their value simply sets them to their default values. |
I agree that the question here is about what should be returned for a default-valued leaf. Since both GET and SUBSCRIBE responses use |
I'm actually arguing that we should address this even earlier, at Replace/Delete time - if a user tries to unset a leaf, and that leaf has a default value, the server should treat this as the user updating that leaf to its default value. Then you don't need any special cases for GET or SUBSCRIBE - as far as they're concerned, a default leaf is no different from any other leaf. |
I said in #142 (comment)
However, I also see in https://github.com/openconfig/reference/blob/master/rpc/gnmi/gnmi-specification.md#332-the-getresponse-message
Therefore, there indeed could be differences between Subscribe and Get. We should clarify if and how they might be different. |
In a streaming subscription What does a delete look like on a leaf with a default value?
|
Can we include a concrete example of such a leaf for discussion? I feel like this is getting into the hypothetical weeds. From a telemetry perspective a leaf either exists or it doesn't. |
A concrete example would be if you issue a DELETE for |
there are two options, as I see it here:
I think both are semantically equivalent, a particular leaf being I have a mild preference for the first here, because it means that the tree size is minimised -- i.e., we do not need to store things in the telemetry collector that have defaults just because they have defaults. |
I think 2. may be more compliant with https://datatracker.ietf.org/doc/html/rfc6020#section-7.6.1. Thoughts?
|
you're right, I think that makes sense. we should add specification clarifications, as well as tests for these decisions. |
There are four independent cases considered below. All examples use the /interfaces/interface/state/enabled leaf, which has a default value of
true
.Question
For all queries below, consider the case that the interface at /interfaces/interface[name="foo"] exists, and that its "enabled" field has its default value of
true
.The question for all of them is, must ("/interfaces/interface[name="foo"]/state/enabled",
true
) be returned by GET?1 Non-wildcard leaf query
e.g. GET /interfaces/interface[name="foo"]/state/enabled
2 Non-wildcard non-leaf query
e.g. GET /interfaces/interface[name="foo"]
3 Wildcard leaf query
e.g. GET /interfaces/interface[name="*"]/state/enabled
4 Wildcard non-leaf query
e.g. GET /interfaces/interface[name="*"]
The text was updated successfully, but these errors were encountered: