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.

Sign up for GitHub

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

Jump to bottom

Clarify wording on SUBSCRIBE/GET to a non-existent path. #144

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

Clarify wording on SUBSCRIBE/GET to a non-existent path. #144

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d7a02f7
Clarify wording on SUBSCRIBE/GET to a non-existent path.
wenovus Sep 10, 2021
83f93c9
Clarify that sync_response is always expected
wenovus Oct 21, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions rpc/gnmi/gnmi-specification.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -917,7 +917,9 @@ The `GetResponse` message consists of:
and hence MUST NOT collapse data from multiple paths into a single
`Notification` within the response. The `timestamp` field of the
`Notification` message MUST be set to the time at which the target's
snapshot of the relevant path was taken.
snapshot of the relevant path was taken. If the path is syntactically valid
but does not (yet) exist, then the `update` field of the `Notification` MUST
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 here the target should respond with an error using the NotFound status code. This seems (to me) to be the most explicit thing to do when a client is explicitly asking for a very particular path.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wouldn't this deviate from Subscribe where no error is produced?

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.

be empty.
* `extension` - a repeated field used to carry gNMI extensions, as per the
description in [Section 2.7](#27-extensions-to-gnmi).

Expand Down Expand Up @@ -1400,8 +1402,10 @@ 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.
would send a `sync_response` as the very first message to indicate this, and in
Copy link
Contributor

Choose a reason for hiding this comment

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

I still don't like this wording.

I suggest:

In the case that a particular path does not (yet) exist, the target should:
 * For `ONCE` subscriptions, return a `sync_response` and close the RPC.
 * For `POLL` subscriptions, return a `sync_response` immediately, and await the next polling request from the client.
 * For `STREAM` subscriptions, if there is no data at a particular path at the start of a subscription, or when a path is sampled, the target should return a `sync_response` immediately. Should the path be created at some later time, the target should begin to transmit updates for the path, according to the specified mode of the subscription.

Thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like it. One question on a previous sentence:

subscribing to any syntactically valid path within such modules

So, if the path is not schema-compliant, it should return NOT_FOUND? i.e. https://pkg.go.dev/google.golang.org/grpc/codes#Code

Should we document this as well?

the case of `STREAM` subscriptions, 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.

<!-- TODO(robjs): Need a way to be able to send this information. Previously
said:
Expand Down