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

[wenovus]

See discussion at #142 (comment)

[dan-lepage]

I think the word "would" ought to be MUST, though I also think this might be clearer to make two different statements for the two different cases, and make it clearer how this applies when you're subscribing to multiple paths, some of which exist and some of which don't. For example, maybe:

When a GET or POLL subscription includes paths that do not (yet) exist in the data tree, the target MUST NOT include those paths in its response (for example, a GET for exclusively paths that don't exist should return a SubscriptionResponse that does not contain any updates).

When a STREAM subscription includes paths that do not (yet) exist in the data tree, the target MUST continue to monitor for the existence of those paths, and transmit telemetry updates should any of them exist in the future. In particular, a STREAM subscription consisting solely of paths that are not in the current data tree MUST NOT close the RPC, but instead should continue monitoring in case those paths are created later on.

[dan-lepage]

One question: By default, a STREAM starts by sending a notification of the current state; I assume in the case that none of the subscribed paths are set it should still send this notification, it just won't contain any updates?

[wenovus]

I'm happy with your modification, although I assume by GET you mean ONCE?

One question: By default, a STREAM starts by sending a notification of the current state; I assume in the case that none of the subscribed paths are set it should still send this notification, it just won't contain any updates?

This is also my understanding from https://github.com/openconfig/reference/blob/master/rpc/gnmi/gnmi-specification.md#3523-sending-telemetry-updates

[dan-lepage]

Oh sorry, yes, I meant ONCE.

[robshakir]

This LGTM, but one question here -- was the clarification that sync_response was not required something that was intended to be removed in the edit.

I can see a structure like:

For subscriptions that exclusively contain paths that do not (yet) exist in the data tree:

• for ONCE
  • target sends no Notifications since there are no values set.
  • target sends sync_response
  • target closes RPC
• for STREAM
  • Target sends no Notifications since there are no values set
  • target sends sync_response
  • target keeps RPC open and monitors for the paths - sending Notifications when they appear.

On the clarification w.r.t whether notifications are sent for unset paths in a STREAM subscription -- if a path isn't set a Notification isn't sent -- i.e., we don't expect to send a notification to say something was nil, so that there is consistent behaviour whether you're in this initial sync or not.

[wenovus]

Ok, updated to say that sync_response is expected as the first message, PTAL.

[dplore]

@gcsl Can you give us your review?

[robshakir]

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.

[wenovus]

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.

[robshakir]

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?

[wenovus]

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?