Re: [Netconf] mbj's WGLC review of subscribed-notifications-16

[Martin Bjorklund <[e-mail]>]

"Eric Voit (evoit)" <[e-mail]> wrote:
> Hi Martin,
> 
> I am fine with the three text clarifications proposed, and will
> incorporate.  More in-line...
> 
> > From: Martin Bjorklund, September 13, 2018 10:21 AM
> > 
> > Hi,
> > 
> > One problematic thing below.
> > 
> > Martin Bjorklund <[e-mail]> wrote:
> > > Hi,
> > >
> > > Thank you for addressing my comments.  More inline.
> > >
> > >
> > > "Eric Voit (evoit)" <[e-mail]> wrote:
> > > > Hi Martin,
> > > >
> > > > Much incorporated.  Latest is at:
> > > > https://github.com/netconf-wg/rfc5277bis/blob/master/draft-ietf-netc
> > > > onf-subscribed-notifications-17.txt
> > > >
> > > > Specific responses in-line...
> > > >
> > > > > From: Martin Bjorklund, September 12, 2018 5:35 AM
> > > > >
> > > > > "Eric Voit (evoit)" <[e-mail]> wrote:
> > > > > > Thanks for the comments Martin,  thoughts in-line...
> > > > > >
> > > > > > > From: Martin Bjorklund, September 10, 2018 1:42 PM
> > > > >
> > > > > [...]
> > > > >
> > > > > > > o  Section 2.1
> > > > > > >
> > > > > > >   The text says:
> > > > > > >
> > > > > > >     It is out of the scope of this document
> > > > > > >     to identify [...] b) how event records are defined
> > > > > > >
> > > > > > >   Is this really what we want?  Shouldn't this document specify
> > > > > > >   that
> > > > > > >   an event record is an instance of a YANG "notification"
> > > > > > >   statement?
> > > > > > >   Do we want to support event records that are not defined in YANG?
> > > > > > >
> > > > > > >   This was discussed in the thread "comments on
> > > > > > >   draft-ietf-netconf-subscribed-notifications-12", and it seems
> > > > > > >   that
> > > > > > >   noone really have any use case for non-YANG-defined event
> > > > > > >   records,
> > > > > > >   accessed with YANG-defined operations.
> > > > > >
> > > > > > Yes this was discussed previously.  My reading was that others
> > > > > > wanted to support subscription to non-YANG events.
> > > > > > E.g., see Andy & Tianran's thoughts in
> > > > > > https://www.ietf.org/mail-archive/web/netconf/current/msg14764.h
> > > > > > tml
> > > > >
> > > > > Yes, but read on in that thread, see
> > > > > https://www.ietf.org/mail-archive/web/netconf/current/msg14766.htm
> > > > > l
> > > > > for a clarification.
> > > > >
> > > > > AFAIK, noone has presented a use case for sending non-YANG-defined
> > > > > data here.
> > > >
> > > > As anything can be YANG wrapped, per the thread above.  I am fine
> > > > with this.
> > > >
> > > > > > >   I think that this document should state that an event record is
> > > > > > >   an
> > > > > > >   instance of a YANG "notification" statement.  If not, it is
> > > > > > >   unclear
> > > > > > >   both how subtree/xpath filters and xml/json/... encodings would
> > work.
> > > > > >
> > > > > > Applying xpath filters against XML based event records seems
> > > > > > viable to me.
> > > > >
> > > > > Where does it say that it has to be XML?  Whatabout mixed-content?
> > > > > What if such a notification is sent when the encoding is JSON?
> > > > >
> > > > > If you want to make this specification open for other data
> > > > > modeling languages, I think you need to carefully define how that
> > > > > is supposed to work; essentially similar to how we handle
> > > > > transports; we have a set of requirement on what a transport
> > > > > document must define.  IMO this is super complicated and overkill.
> > > > > This draft is complex enough.
> > > > > Don't add even more complexity.
> > > >
> > > > If you are just saying that we can put everything into a YANG
> > > > wrapper, that is fine with me.  To clarify this, I have tweaked the
> > > > first sentence of Section 2.1 to:
> > > > "An event stream is a named entity on a publisher which exposes a
> > > > continuously updating set of YANG encoded event records."
> > >
> > > Howabout:
> > >
> > > OLD:
> > >
> > >    An event stream is a named entity on a publisher which exposes a
> > >    continuously updating set of event records.  Each event stream is
> > >    available for subscription.  It is out of the scope of this document
> > >    to identify a) how streams are defined (other than the NETCONF
> > >    stream), b) how event records are defined/generated, and c) how event
> > >    records are assigned to streams.
> > >
> > > NEW:
> > >
> > >    An event stream is a named entity on a publisher which exposes a
> > >    continuously updating set of event records.  An event record is
> > >    defined with the "notification" YANG statement.  Each event stream is
> > >    available for subscription.  It is out of the scope of this document
> > >    to identify a) how streams are defined (other than the NETCONF
> > >    stream), b) how event records are generated, and c) how event
> > >    records are assigned to streams.
> > >
> > > > And I have tweaked the first sentence of the third paragraph to:
> > > > "As YANG encoded event records are created by a system,..."
> > >
> > > I think the original sentence is better.
> > >
> > > Also, some tweaks are needed to the filter leafs in the YANG module:
> > >
> > > OLD:
> > >
> > >           The subtree filter is applied to the representation of
> > >           individual, delineated event records as contained within the
> > >           event stream.  For example, if the notification message
> > >           contains an instance of a notification defined in YANG, then
> > >           the top-level element is the name of the YANG notification.
> > >
> > > NEW:
> > >
> > >           The subtree filter is applied to the representation of
> > >           individual, delineated event records as contained within the
> > >           event stream.  The top-level element in the event record is
> > >           the name of the YANG notification.
> > >
> > > and
> > >
> > > OLD:
> > >
> > >            The XPath expression is evaluated on the representation of
> > >            individual, delineated event records as contained within
> > >            the event stream.  For example, if the notification message
> > >            contains an instance of a notification defined in YANG,
> > >            then the top-level element is the name of the YANG
> > >            notification, and the root node has this top-level element
> > >            as the only child.
> > >
> > > NEW:
> > >
> > >
> > >            The XPath expression is evaluated on the representation of
> > >            individual, delineated event records as contained within
> > >            the event stream.  The top-level element in the event
> > >            record is the name of the YANG notification, and the root
> > >            node has this top-level element as the only child.
> > 
> > 
> > This text is problematic b/c it doesn't work well with inline
> > notifications.
> > 
> > Consider the example in section 7.16.3 in RFC 7950:
> > 
> >        container interfaces {
> >          list interface {
> >            key "name";
> >            leaf name {
> >              type string;
> >            }
> >            notification interface-enabled {
> >              leaf by-user {
> >                type string;
> >              }
> >            }
> >          }
> >        }
> > 
> > and corresponding XML example:
> > 
> >      <notification
> >        xmlns="urn:ietf:params:xml:ns:netconf:notification:1.0">
> >        <eventTime>2008-07-08T00:01:00Z</eventTime>
> >        <interfaces xmlns="urn:example:interface-module">
> >          <interface>
> >            <name>eth1</name>
> >            <interface-enabled>
> >              <by-user>fred</by-user>
> >            </interface-enabled>
> >          </interface>
> >        </interfaces>
> >      </notification>
> > 
> > What, in this example, is the "event record"?  I assume it is this
> > part:
> > 
> >        <interfaces xmlns="urn:example:interface-module">
> >          <interface>
> >            <name>eth1</name>
> >            <interface-enabled>
> >              <by-user>fred</by-user>
> >            </interface-enabled>
> >          </interface>
> >        </interfaces>
> > 
> > But the element that is the name of the YANG notification is this:
> > 
> >            <interface-enabled>
> >              <by-user>fred</by-user>
> >            </interface-enabled>
> > 
> > 
> > (With draft-ietf-netconf-notification-message, the "event record"
> > would be placed in the <notification-contents> anydata)
> > 
> > 
> > A problem is that the containment hierarchy is defined in RFC 7950 in
> > the XML
> > encoding text.  There is no text in RFC 7951 or RFC 8040 for how to do
> > this in
> > JSON for RESTCONF.  It is natural to do a pure JSON-translation of the
> > XML
> > example, but this isn't specified anywhere.
> > 
> > 
> > So I propose the following tweaked changes:
> > 
> > 
> > OLD:
> > 
> >    An event stream is a named entity on a publisher which exposes a
> >    continuously updating set of event records.  Each event stream is
> >    available for subscription.  It is out of the scope of this document
> >    to identify a) how streams are defined (other than the NETCONF
> >    stream), b) how event records are defined/generated, and c) how event
> >    records are assigned to streams.
> > 
> > NEW:
> > 
> >    An event stream is a named entity on a publisher which exposes a
> >    continuously updating set of event records.  An event record is an
> >    intantiation of a "notification" YANG statement.  If the
> >    "notification" is defined as a child to a data node, the
> >    intantiation includes the hierarchy of nodes that identifies the
> >    data node in the datastore (see Section 7.16.2 of RFC 7950).  Each
> >    event stream is available for subscription.  It is out of the scope
> >    of this document to identify a) how streams are defined (other than
> >    the NETCONF stream), b) how event records are generated, and c) how
> >    event records are assigned to streams.
> > 
> > 
> > and in the YANG module:
> > 
> > OLD:
> > 
> >           The subtree filter is applied to the representation of
> >           individual, delineated event records as contained within the
> >           event stream.  For example, if the notification message
> >           contains an instance of a notification defined in YANG, then
> >           the top-level element is the name of the YANG notification.
> > 
> > NEW:
> > 
> >           The subtree filter is applied to the representation of
> >           individual, delineated event records as contained within the
> >           event stream.
> > 
> > and
> > 
> > OLD:
> > 
> >            The XPath expression is evaluated on the representation of
> >            individual, delineated event records as contained within
> >            the event stream.  For example, if the notification message
> >            contains an instance of a notification defined in YANG,
> >            then the top-level element is the name of the YANG
> >            notification, and the root node has this top-level element
> >            as the only child.
> > 
> > NEW:
> > 
> > 
> >            The XPath expression is evaluated on the representation of
> >            individual, delineated event records as contained within
> >            the event stream.
> > 
> > > (I think it would be useful with some filtering examples, like in
> > > section 5 of RFC 5277, but I think you have all examples in
> > > netconf-notif, so they would go into that draft.  (re-reading an old
> > > thread I found that examples for filters were requested before)).
> >
> > And I strongly think that we should add examples in an appendix, even
> > though
> > this is a protocol and encoding neutral document.  Maybe one example
> > with
> > NETCONF XML and one with RESTCONF JSON.  One example can show a
> > subtree filter and a top-level notif, and the other an XPath filter
> > with an inline
> > notif.
> 
> We have worked very hard to keep the document protocol agnostic.  Even
> the Appendix A example protocol for configured subscriptions is 'foo'.
> 
> If you are ok with it, I am completely fine with adding two such
> NETCONF XML filtering examples into a new Appendix section A.4 in
> NETCONF-Notif.  Perhaps Reshad would be able to do the same for
> RESTCONF JSON within RESTCONF-Notif.

I think that having an example in one encoding for one protocol (or
two of them) wouldn't make the document less protocol agnostic.  For
example RFC 8342 (the NMDA architecture document) has some XML
examples.

See also section 3.12 of rfc6087bis.  (at least once I added examples
to a document during the AD review).

Examples also help verify that the text is clear.

This said, I won't object to the document moving forward w/o
examples.


/martin