Hi,
Andy Bierman <andy@xxxxxxxxxxxxx> wrote:
[...]
> 2.4.2.1. Replay Subscription
>
> If the "replay-start-
> time" contains a value that is earlier than content stored within the
> publisher's replay buffer, then the subscription MUST be rejected,
> and the leaf "replay-start-time-hint" MUST be set in the reply.
>
> >> this is a significant and bad change from RFC 5277 behavior
> >> the start-time says "send all events that you have stored
> since this time" The server sends its oldest event and does
> not reject the request. This draft incorrectly interprets
> the request as "the server MUST have an event stored at least
> this old"
I agree, and I have pointed this out in earlier reviews.
If a client sends a too early time, and the server doesn't send the
optional hint, the client will have to guess the time. Not very
robust.
If the motivation is that the client should be informed that he might
have missed some notifs b/c the replay-start-time is too early, this
information can be passed in the rpc-reply from establish-subscription
instead.
> If a "stop-time" parameter is included, it MAY also be earlier than
> the current time and MUST be later than the "replay-start-time". The
> publisher MUST NOT accept a "replay-start-time" for a future time.
>
> >> MUST be later (if the start-time) if supplied
> >> MAY be before current time? Inconsistent with start-time
> MUST have events that exist
> >> MUST NOT accept future start-time different than 5277, but OK
> because that was a bad requirement
I have also pointed out in earlier reviews that the requirement that
the replay-start-time cannot be in the future is problematic:
I know that this text is also present in RFC 5277, but I think it
needs to be changed. Which current time? Probably the server's,
but how would a client know that? This is a problem that we faced
when implementing 5277. I think we should remove this
requirement, since it doesn't add any value anyway.
IMO, if the server gets a replay-start-time that is later than the
latest stored notif, it will send a <replay-completed> notif, and then
move on. This is a very simple behavior that doesn't rely on
synchronized clocks or anything like that.
[...]
> 2.5.2. Creating a Configured Subscription
>
> In this case, when there is
> something to transport for an active subscription, transport specific
> call-home operations will be used to establish the connection. When
>
> >> is this normative or is callhome optional-to-implement?
>
> With active configured subscriptions, it is allowable to buffer event
> records even after a "subscription-started" has been sent. However
> if events are lost (rather than just delayed) due to replay buffer
> overflow, a new "subscription-started" must be sent. This new
> "subscription-started" indicates an event record discontinuity.
>
> >> this is confusing to send multiple "subscription-started" events.
>
> To see an example at subscription creation using configuration
> operations over NETCONF, see Appendix A of
> [I-D.draft-ietf-netconf-netconf-event-notifications].
>
> >> IMO the examples should be moved to this draft
+1
[...]
> 2.7.1. subscription-started
> 2.7.2. subscription-modified
>
> >> what is the value of returning all the input or configuration
> parameters in these notifications? For a dynamic subscription,
> the only receiver just sent that info and does not need it.
> For a configured subscription, that data can be read from
> the configuration datastore.
I agree. Removing these redundant parameters would also simplify the
models quite a bit.
[...]
> 4A) message encoding
>
> feature encode-json {
> description
> "This feature indicates that JSON encoding of notification
> messages is supported.";
> }
>
> feature encode-xml {
> description
> "This feature indicates that XML encoding of notification
> messages is supported.";
> }
>
> identity encodings {
> description
> "Base identity to represent data encodings";
> }
>
> identity encode-xml {
> base encodings;
> if-feature "encode-xml";
> description
> "Encode data using XML";
> }
>
> identity encode-json {
> base encodings;
> if-feature "encode-json";
> description
> "Encode data using JSON";
> }
>
> typedef encoding {
> type identityref {
> base encodings;
> }
> description
> "Specifies a data encoding, e.g. for a data subscription.";
> }
>
> leaf encoding {
> type encoding;
> mandatory true;
> description
> "The type of encoding for the subscribed data.";
> }
>
> >> IMO all YANG definitions related to message encoding should
> be removed because they are in conflict with existing protocols.
> NETCONF defines XML encoding. HTTP already defines
> media type handling for message encoding (Accept, Content-Type)
> There is no definition how to use JSON with NETCONF.
+1
/martin
