Subject: Re: [yang-doctors] [netconf] Yangdoctors last call review of draft-ietf-netconf-yang-push

From: Kent Watsen <[email-masked]>
Date: Tue, 5 Feb 2019 01:45:23 +0000

[-netconf list]

Hi Authors,

Just want to confirm that you received this review and are working on a response.

Thanks,
Kent

> On Jan 30, 2019, at 10:51 AM, Martin Bjorklund <[email-masked]> wrote:
> 
> Hi,
> 
> This is my YANG doctor's review of draft-ietf-netconf-yang-push-21.
> 
> Reviewer: Martin Björklund
> Review result: Ready with Issues
> 
> 
> o  General
> 
>  Check the output of idnits.  Specifically, add the 2119
>  boilerplate.
> 
>  Also, since the YANG module uses 2119 language, add the YANG-ified
>  version of the boilerplate to the description statement in the
>  module, just before the copyright.
> 
> 
> o  General
> 
>  The module is inconsistently indented and formatted.  The RFC editor
>  will enforce consistent formatting, so in order to make their life
>  easier it is better to do this work before it reaches them.
> 
>  Attached is a cleaned up version of the YANG module where I have
>  done the following changes:
> 
>    o  pyang -f yang --keep-comments --yang-line-length 69
> 
>       (requires pyang 1.7.8)
> 
>    o  added newlines in long descriptions, to make separate
>       paragraphs
> 
>    o  added extra space after "." in description statement (most of
>       them already had two spaces)
> 
>    o  formatted some paragraphs (I use fill-paragraph in emacs, manually)
> 
>    o  added the 2119 boilerplate
> 
>  NOTE: I haven't done any other of the changes that I propose in
>  this review.
> 
> 
> o  Figure 6
> 
>  The tree diagram for push-change-update doesn't match the module.
> 
>  OLD:
> 
>     notifications:
>       +---n push-update
>       |  +--ro id?      sn:subscription-id
>       |  +--ro datastore-contents?   <anydata>
>       |  +--ro incomplete-update?     empty
>       +---n push-change-update {on-change}?
>          +--ro id?     sn:subscription-id
>          +--ro datastore-changes?
>          |  +--ro yang-patch
>          |     +--ro patch-id        string
>          |     +--ro ypatch:comment?    string
>          |     +--ro ypatch:edit* [edit-id]
>          |        +--ro ypatch:edit-id      string
>          |        +--ro ypatch:operation    enumeration
>          |        +--ro ypatch:target       target-resource-offset
>          |        +--ro ypatch:point?       target-resource-offset
>          |        +--ro ypatch:where?       enumeration
>          |        +--ro ypatch:value?
>          +--ro incomplete-update?    empty
> 
>  NEW:
> 
>     notifications:
>       +---n push-update
>       |  +--ro id?                   sn:subscription-id
>       |  +--ro datastore-contents?   <anydata>
>       |  +--ro incomplete-update?    empty
>       +---n push-change-update {on-change}?
>          +--ro id?                  sn:subscription-id
>          +--ro datastore-changes
>          |  +--ro yang-patch
>          |     +--ro patch-id    string
>          |     +--ro comment?    string
>          |     +--ro edit* [edit-id]
>          |        +--ro edit-id      string
>          |        +--ro operation    enumeration
>          |        +--ro target       target-resource-offset
>          |        +--ro point?       target-resource-offset
>          |        +--ro where?       enumeration
>          |        +--ro value?       <anydata>
>          +--ro incomplete-update?   empty
> 
> 
> o  Figure 6
> 
>  The purpose of including tree diagrams is to give a easy-to-read
>  overview to the reader.  When the diagram is spread out over
>  several pages, it becomes more difficult to grasp.
> 
>  In this case, I would suggest that you split the diagram into
>  smaller pieces, and place them together with the text that describe
>  them.
> 
>  For example, in 4.2 you can have the first part of the tree that
>  deals with subscription config; 4.3.1 the augmented notifs etc.
> 
> 
> o  Figure 9
> 
>  This example is difficult to read.  I suggest:
> 
>  OLD:
> 
>   <netconf:rpc message-id="101"
>     xmlns:netconf="urn:ietf:params:xml:ns:netconf:base:1.0">
>     <establish-subscription
>     xmlns="urn:ietf:params:xml:ns:yang:ietf-subscribed-notifications"
>     xmlns:yp="urn:ietf:params:xml:ns:yang:ietf-yang-push">
>       <yp:datastore
>       xmlns:ds="urn:ietf:params:xml:ns:yang:ietf-datastores">
>         ds:operational
>       </yp:datastore>
>       <yp:datastore-xpath-filter
>       xmlns:ex="http://example.com/sample-data/1.0">
>         /ex:foo
>       </yp:datastore-xpath-filter>
>       <yp:on-change>
>         <yp:dampening-period>100</yp:dampening-period>
>       </yp:on-change>
>     </establish-subscription>
>   </netconf:rpc>
> 
>  NEW:
> 
>   <rpc message-id="101"
>        xmlns="urn:ietf:params:xml:ns:netconf:base:1.0">
>     <establish-subscription
>         xmlns=
>           "urn:ietf:params:xml:ns:yang:ietf-subscribed-notifications"
>         xmlns:yp="urn:ietf:params:xml:ns:yang:ietf-yang-push">
>       <yp:datastore
>           xmlns:ds="urn:ietf:params:xml:ns:yang:ietf-datastores">
>         ds:operational
>       </yp:datastore>
>       <yp:datastore-xpath-filter
>           xmlns:ex="http://example.com/sample-data/1.0">
>         /ex:foo
>       </yp:datastore-xpath-filter>
>       <yp:on-change>
>         <yp:dampening-period>100</yp:dampening-period>
>       </yp:on-change>
>     </establish-subscription>
>   </rpc>
> 
> 
> o  Figure 11
> 
>  Similar in this example:
> 
>  OLD:
> 
>   <netconf:rpc message-id="102"
>      xmlns:netconf="urn:ietf:params:xml:ns:netconf:base:1.0">
>      <modify-subscription
>      xmlns="urn:ietf:params:xml:ns:yang:ietf-subscribed-notifications"
>      xmlns:yp="urn:ietf:params:xml:ns:yang:ietf-yang-push">
>       <id>1011</id>
>       <yp:datastore
>       xmlns:ds="urn:ietf:params:xml:ns:yang:ietf-datastores">
>         ds:operational
>       </yp:datastore>
>       <yp:datastore-xpath-filter
>         xmlns:ex="http://example.com/sample-data/1.0">
>         /ex:bar
>       </yp:datastore-xpath-filter>
>       <yp:periodic>
>         <yp:period>250</yp:period>
>       </yp:periodic>
>      </modify-subscription>
>   </netconf:rpc>
> 
>  NEW:
> 
>   <rpc message-id="102"
>        xmlns="urn:ietf:params:xml:ns:netconf:base:1.0">
>     <modify-subscription
>         xmlns=
>           "urn:ietf:params:xml:ns:yang:ietf-subscribed-notifications"
>         xmlns:yp="urn:ietf:params:xml:ns:yang:ietf-yang-push">
>       <id>1011</id>
>       <yp:datastore
>           xmlns:ds="urn:ietf:params:xml:ns:yang:ietf-datastores">
>         ds:operational
>       </yp:datastore>
>       <yp:datastore-xpath-filter
>           xmlns:ex="http://example.com/sample-data/1.0">
>         /ex:bar
>       </yp:datastore-xpath-filter>
>       <yp:periodic>
>         <yp:period>250</yp:period>
>       </yp:periodic>
>      </modify-subscription>
>   </rpc>
> 
> 
> o  Section 4.4.4
> 
>  (The figure in this example doesn't have a number)
> 
>  OLD:
> 
> <netconf:rpc message-id="103"
> xmlns:netconf="urn:ietf:params:xml:ns:netconf:base:1.0">
>   <resync-subscription
>   xmlns="urn:ietf:params:xml:ns:yang:ietf-yang-push"
>   xmlns:sn="urn:ietf:params:xml:ns:yang:ietf-subscribed-notifications">
>     <id>1011</id>
>   </resync-subscription>
> </netconf:rpc>
> 
>  NEW:
> 
>   <rpc message-id="103"
>        xmlns="urn:ietf:params:xml:ns:netconf:base:1.0">
>     <resync-subscription
>         xmlns="urn:ietf:params:xml:ns:yang:ietf-yang-push"
>       <id>1011</id>
>     </resync-subscription>
>    </netconf:rpc>
> 
> o  Section 5.
> 
>  In order to get the RFC references right,  follow the pattern in
>  other documents with YANG modules; e.g. RFC 8343.  Specifically,
>  all references in RFCs must be found in the main text; this means
>  that you need to add something like:
> 
>   This YANG module imports typedefs from [RFC6991].
> 
> 
> o  import yang-patch
> 
>  OLD:
> 
>    reference
>      "RFC 8072: YANG Patch";
> 
>  NEW:
> 
>    reference
>      "RFC 8072: YANG Patch Media Type";
> 
> 
> o  typedef change-type
> 
>  The "reference" statement contains text that should be in the
>  "description" statement (this comment has been raised before on the
>  ML).
> 
>  I suggest:
> 
>  OLD:
> 
>    description
>      "Specifies different types of datastore changes.";
>    reference
>      "RFC 8072 section 2.5, with a delta that it is valid for a
>       receiver to process an update record which performs a create
>       operation on a datastore node the receiver believes exists,
>       or to process a delete on a datastore node the receiver
>       believes is missing.";
> 
>   NEW:
> 
>    description
>      "Specifies different types of datastore changes.
> 
>       This type is based on the edit operations defined for YANG
>       Patch, with the difference that it is valid for a receiver to
>       process an update record which performs a create operation on
>       a datastore node the receiver believes exists, or to process a
>       delete on a datastore node the receiver believes is missing.";
>    reference
>      "RFC 8072: YANG Patch Media Type, section 2.5";
> 
> 
> o  wrong type used (timeticks)
> 
>  This was discussed on the ML, but apparently the change wasn't
>  introduced yet.
> 
>  A couple of 'period' nodes use the type yang:timeticks, but it is
>  not an appropriate type.  RFC 6991 describes it as:
> 
>         The timeticks type represents a non-negative integer that
>         represents the time, modulo 2^32 (4294967296 decimal), in
>         hundredths of a second between two epochs.  When a schema
>         node is defined that uses this type, the description of
>         the schema node identifies both of the reference epochs.
> 
>  As Juergen pointed out, it seems you want a TimeInterval - but this
>  type is not present in RFC 6991.  So add:
> 
>    type centiseconds {
>      type uint32;
>      description
>        "A period of time, measured in units of 0.01 seconds.";
>    }
> 
>  and use that instead.
> 
> 
> o  container datastore-changes
> 
>  OLD:
> 
>    container datastore-changes {
>      description
>        "This contains the set of datastore changes of the target
>         datastore starting at the time of the previous update, per
>         the terms of the subscription.  The datastore changes are
>         encoded per RFC 8027 (YANG Patch).";
>      uses ypatch:yang-patch;
> 
>  NEW:
> 
>    container datastore-changes {
>      description
>        "This contains the set of datastore changes of the target
>         datastore starting at the time of the previous update, per
>         the terms of the subscription.";
>      uses ypatch:yang-patch;
> 
>   (The reference was not correct, and it isn't really necessary
>   anymore, since you use the grouping from yang patch.)
> 
> 
> o  leaf datastore-xpath-filter
> 
>  This leaf has the same problem that stream-xpath-filter had; it
>  doesn't work for RESTCONF.  We need to apply the same solution as
>  for stream-xpath-filter:
> 
>  OLD:
> 
>       leaf datastore-xpath-filter {
>         if-feature "sn:xpath";
>         type yang:xpath1.0;
>         description
>           "This parameter contains an XPath expression identifying
>           the portions of the target datastore to retrieve.
>           If the expression returns a node-set, all nodes in the
>           node-set are selected by the filter.  Otherwise, if the
>           expression does not return a node-set, the filter
>           doesn't select any nodes.
>           The expression is evaluated in the following XPath
>           context:
>            o  The set of namespace declarations are those in scope
>               on the 'datastore-xpath-filter' leaf element.
>            o  The set of variable bindings is empty.
>            o  The function library is the core function library, and
>               the XPath functions defined in section 10 in RFC 7950.
>            o  The context node is the root node of the target
>               datastore.";
>       }
> 
>  NEW:
> 
>       leaf datastore-xpath-filter {
>         if-feature "sn:xpath";
>         type yang:xpath1.0;
>         description
>           "This parameter contains an XPath expression identifying
>           the portions of the target datastore to retrieve.
> 
>           If the expression returns a node-set, all nodes in the
>           node-set are selected by the filter.  Otherwise, if the
>           expression does not return a node-set, the filter
>           doesn't select any nodes.
> 
>           The expression is evaluated in the following XPath
>           context:
> 
>             o   The set of namespace declarations is the set of prefix
>                 and namespace pairs for all YANG modules implemented
>                 by the server, where the prefix is the YANG module
>                 name and the namespace is as defined by the
>                 'namespace' statement in the YANG module.
> 
>                 If the leaf is encoded in XML, all namespace
>                 declarations in scope on the 'stream-xpath-filter'
>                 leaf element are added to the set of namespace
>                 declarations.  If a prefix found in the XML is
>                 already present in the set of namespace declarations,
>                 the namespace in the XML is used.
> 
>             o  The set of variable bindings is empty.
> 
>             o  The function library is the core function library, and
>                the XPath functions defined in section 10 in RFC 7950.
> 
>             o  The context node is the root node of the target
>                datastore.";
>       }
> 
> 
> o  Section A.1
> 
>  I have trouble parsing this paragraph:
> 
>   References to specific identities within the either the subscribed-
>   notifications YANG model or the yang-push YANG model may be returned
>   as part of the error responses resulting from failed attempts at
>   datastore subscription.  Following are valid errors per RPC (note:
>   throughout this section the prefix 'sn' indicates an item imported
>   from the subscribed-notifications.yang model):
> 
>  "the either the" needs to be fixed.
> 
>  Suggest you spell ou the real names of the YANG modules you refer
>  to (s/subscribed-notifications.yang
>  model/"ietf-subscribed-notifications" YANG module/  etc)
> 
> 
> o  Section A.1
> 
>  I don't think this document should repeat what's already specified
>  in the SN draft.  Specifically, I think this section should list
>  the *additional* errors that this document introduces.  I also note
>  that the list in this document is not the same as in the SN draft...
> 
>  So I suggest:
> 
>  OLD:
> 
>   establish-subscription         modify-subscription
>   ----------------------         -------------------
>    cant-exclude                   sn:filter-unsupported
>    datastore-not-subscribable     sn:insufficient-resources
>    sn:dscp-unavailable            sn:no-such-subscription
>    sn:filter-unsupported          period-unsupported
>    sn:insufficient-resources      update-too-big
>    on-change-unsupported          sync-too-big
>    on-change-sync-unsupported     unchanging-selection
>    period-unsupported
>    update-too-big                resync-subscription
>    sync-too-big                  --------------------
>    unchanging-selection           no-such-subscription-resync
>                                   sync-too-big
> 
>   delete-subscription            kill-subscription
>   ----------------------         -----------------
>    sn:no-such-subscription        sn:no-such-subscription
> 
>  NEW:
> 
>   establish-subscription         modify-subscription
>   ----------------------         -------------------
>    cant-exclude                   period-unsupported
>    datastore-not-subscribable     update-too-big
>    on-change-unsupported          sync-too-big
>    on-change-sync-unsupported     unchanging-selection
>    period-unsupported
>    update-too-big                resync-subscription
>    sync-too-big                  --------------------
>    unchanging-selection           no-such-subscription-resync
>                                   sync-too-big
> 
> 
> 
> o  Section A.2
> 
>  Similar to A.1, suggest you remove the sn: errors that are already
>  covered by SN.
> 
> 
> 
> 
> /martin
> module ietf-yang-push {
>  yang-version 1.1;
>  namespace "urn:ietf:params:xml:ns:yang:ietf-yang-push";
>  prefix yp;
> 
>  import ietf-yang-types {
>    prefix yang;
>    reference
>      "RFC 6991: Common YANG Data Types";
>  }
>  import ietf-subscribed-notifications {
>    prefix sn;
>    reference
>      "draft-ietf-netconf-subscribed-notifications:
>       Customized Subscriptions to a Publisher's Event Streams
>       NOTE TO RFC Editor: Please replace above reference to
>       draft-ietf-netconf-subscribed-notifications with RFC number
>       when published (i.e. RFC xxxx).";
>  }
>  import ietf-datastores {
>    prefix ds;
>    reference
>      "RFC 8342: Network Management Datastore Architecture (NMDA)";
>  }
>  import ietf-restconf {
>    prefix rc;
>    reference
>      "RFC 8040: RESTCONF Protocol";
>  }
>  import ietf-yang-patch {
>    prefix ypatch;
>    reference
>      "RFC 8072: YANG Patch";
>  }
> 
>  organization
>    "IETF NETCONF Working Group";
>  contact
>    "WG Web:   <http://tools.ietf.org/wg/netconf/>
> 
>     WG List:  <mailto:[email-masked]>
> 
>     Editor:   Alexander Clemm
>               <mailto:[email-masked]>
>     Editor:   Eric Voit
>               <mailto:[email-masked]>
>     Editor:   Alberto Gonzalez Prieto
>               <mailto:[email-masked]>
>     Editor:   Ambika Prasad Tripathy
>               <mailto:[email-masked]>
>     Editor:   Einar Nilsen-Nygaard
>               <mailto:[email-masked]>
>     Editor:   Andy Bierman
>               <mailto:[email-masked]>
>     Editor:   Balazs Lengyel
>               <mailto:[email-masked]>";
>  description
>    "This module contains YANG specifications for YANG push.
> 
>     The key words 'MUST', 'MUST NOT', 'REQUIRED', 'SHALL', 'SHALL
>     NOT', 'SHOULD', 'SHOULD NOT', 'RECOMMENDED', 'NOT RECOMMENDED',
>     'MAY', and 'OPTIONAL' in this document are to be interpreted as
>     described in BCP 14 (RFC 2119) (RFC 8174) when, and only when,
>     they appear in all capitals, as shown here.
> 
>     Copyright (c) 2019 IETF Trust and the persons identified as
>     authors of the code.  All rights reserved.
> 
>     Redistribution and use in source and binary forms, with or
>     without modification, is permitted pursuant to, and subject to
>     the license terms contained in, the Simplified BSD License set
>     forth in Section 4.c of the IETF Trust's Legal Provisions
>     Relating to IETF Documents
>     (https://trustee.ietf.org/license-info).
> 
>     This version of this YANG module is part of RFC XXXX;
>     see the RFC itself for full legal notices.";
> 
>   // RFC Ed.: replace XXXX with actual RFC number and remove this
>   // note.
> 
>  revision 2019-01-29 {
>    description
>      "Initial revision.
>       NOTE TO RFC EDITOR:
>       (1)Please replace the above revision date to
>       the date of RFC publication when published.
>       (2) Please replace the date in the file name
>       ([email-masked]) to the date of RFC
>       publication.
>       (3) Please replace the following reference to
>       draft-ietf-netconf-yang-push-21 with RFC number when
>       published (i.e. RFC xxxx).";
>    reference
>      "draft-ietf-netconf-yang-push-21";
>  }
> 
>  /*
>   * FEATURES
>   */
> 
>  feature on-change {
>    description
>      "This feature indicates that on-change triggered subscriptions
>       are supported.";
>  }
> 
>  /*
>   * IDENTITIES
>   */
> 
>  /* Error type identities for datastore subscription  */
> 
>  identity resync-subscription-error {
>    description
>      "Problem found while attempting to fulfill an
>       'resync-subscription' RPC request.";
>  }
> 
>  identity cant-exclude {
>    base sn:establish-subscription-error;
>    description
>      "Unable to remove the set of 'excluded-changes'.  This means
>       the publisher is unable to restrict 'push-change-update's to
>       just the change types requested for this subscription.";
>  }
> 
>  identity datastore-not-subscribable {
>    base sn:establish-subscription-error;
>    base sn:subscription-terminated-reason;
>    description
>      "This is not a subscribable datastore.";
>  }
> 
>  identity no-such-subscription-resync {
>    base resync-subscription-error;
>    description
>      "Referenced subscription doesn't exist. This may be as a result
>       of a non-existent subscription ID, an ID which belongs to
>       another subscriber, or an ID for configured subscription.";
>  }
> 
>  identity on-change-unsupported {
>    base sn:establish-subscription-error;
>    description
>      "On-change is not supported for any objects which are
>       selectable by this filter.";
>  }
> 
>  identity on-change-sync-unsupported {
>    base sn:establish-subscription-error;
>    description
>      "Neither sync on start nor resynchronization are supported for
>       this subscription.  This error will be used for two
>       reasons.  First if an 'establish-subscription' RPC includes
>       'sync-on-start', yet the publisher can't support sending a
>       'push-update' for this subscription for reasons other than
>       'on-change-unsupported' or 'sync-too-big'.  And second, if the
>       'resync-subscription' RPC is invoked either for an existing
>       periodic subscription, or for an on-change subscription which
>       can't support resynchronization.";
>  }
> 
>  identity period-unsupported {
>    base sn:establish-subscription-error;
>    base sn:modify-subscription-error;
>    base sn:subscription-suspended-reason;
>    description
>      "Requested time period or dampening-period is too short.  This
>       can be for both periodic and on-change subscriptions (with or
>       without dampening.) Hints suggesting alternative periods may
>       be returned as supplemental information.";
>  }
> 
>  identity update-too-big {
>    base sn:establish-subscription-error;
>    base sn:modify-subscription-error;
>    base sn:subscription-suspended-reason;
>    description
>      "Periodic or on-change push update datatrees exceed a maximum
>       size limit.  Hints on estimated size of what was too big may
>       be returned as supplemental information.";
>  }
> 
>  identity sync-too-big {
>    base sn:establish-subscription-error;
>    base sn:modify-subscription-error;
>    base resync-subscription-error;
>    base sn:subscription-suspended-reason;
>    description
>      "Sync-on-start or resynchronization datatree exceeds a maximum
>       size limit.  Hints on estimated size of what was too big may
>       be returned as supplemental information.";
>  }
> 
>  identity unchanging-selection {
>    base sn:establish-subscription-error;
>    base sn:modify-subscription-error;
>    base sn:subscription-terminated-reason;
>    description
>      "Selection filter is unlikely to ever select datatree nodes.
>       This means that based on the subscriber's current access
>       rights, the publisher recognizes that the selection filter is
>       unlikely to ever select datatree nodes which change.  Examples
>       for this might be that node or subtree doesn't exist, read
>       access is not permitted for a receiver, or static objects that
>       only change at reboot have been chosen.";
>  }
> 
>  /*
>   * TYPE DEFINITIONS
>   */
> 
>  typedef change-type {
>    type enumeration {
>      enum create {
>        description
>          "A change that refers to the creation of a new datastore
>           node.";
>      }
>      enum delete {
>        description
>          "A change that refers to the deletion of a datastore
>           node.";
>      }
>      enum insert {
>        description
>          "A change that refers to the insertion of a new
>           user-ordered datastore node.";
>      }
>      enum move {
>        description
>          "A change that refers to a reordering of the target
>           datastore node.";
>      }
>      enum replace {
>        description
>          "A change that refers to a replacement of the target
>           datastore node's value.";
>      }
>    }
>    description
>      "Specifies different types of datastore changes.";
>    reference
>      "RFC 8072 section 2.5, with a delta that it is valid for a
>       receiver to process an update record which performs a create
>       operation on a datastore node the receiver believes exists,
>       or to process a delete on a datastore node the receiver
>       believes is missing.";
>  }
> 
>  typedef selection-filter-ref {
>    type leafref {
>      path "/sn:filters/yp:selection-filter/yp:filter-id";
>    }
>    description
>      "This type is used to reference a selection filter.";
>  }
> 
>  /*
>   * GROUP DEFINITIONS
>   */
> 
>  grouping datastore-criteria {
>    description
>      "A grouping to define criteria for which selected objects
>       from  a targeted datastore should be included in push
>       updates.";
>    leaf datastore {
>      type identityref {
>        base ds:datastore;
>      }
>      mandatory true;
>      description
>        "Datastore from which to retrieve data.";
>    }
>    uses selection-filter-objects;
>  }
> 
>  grouping selection-filter-types {
>    description
>      "This grouping defines the types of selectors for objects
>       from a datastore.";
>    choice filter-spec {
>      description
>        "The content filter specification for this request.";
>      anydata datastore-subtree-filter {
>        if-feature "sn:subtree";
>        description
>          "This parameter identifies the portions of the
>           target datastore to retrieve.";
>        reference
>          "RFC 6241: Network Configuration Protocol, Section 6.";
>      }
>      leaf datastore-xpath-filter {
>        if-feature "sn:xpath";
>        type yang:xpath1.0;
>        description
>          "This parameter contains an XPath expression identifying
>           the portions of the target datastore to retrieve.
> 
>           If the expression returns a node-set, all nodes in the
>           node-set are selected by the filter.  Otherwise, if the
>           expression does not return a node-set, the filter
>           doesn't select any nodes.
> 
>           The expression is evaluated in the following XPath
>           context:
> 
>           o  The set of namespace declarations are those in scope
>              on the 'datastore-xpath-filter' leaf element.
>           o  The set of variable bindings is empty.
>           o  The function library is the core function library, and
>              the XPath functions defined in section 10 in RFC 7950.
>           o  The context node is the root node of the target
>              datastore.";
>      }
>    }
>  }
> 
>  grouping selection-filter-objects {
>    description
>      "This grouping defines a selector for objects from a
>       datastore.";
>    choice selection-filter {
>      description
>        "The source of the selection filter applied to the
>         subscription.  This will come either referenced from a global
>         list, or be provided within the subscription itself.";
>      case by-reference {
>        description
>          "Incorporate a filter that has been configured
>           separately.";
>        leaf selection-filter-ref {
>          type selection-filter-ref;
>          mandatory true;
>          description
>            "References an existing selection filter which is to be
>             applied to the subscription.";
>        }
>      }
>      case within-subscription {
>        description
>          "Local definition allows a filter to have the same
>           lifecycle as the subscription.";
>        uses selection-filter-types;
>      }
>    }
>  }
> 
>  grouping update-policy-modifiable {
>    description
>      "This grouping describes the datastore specific subscription
>       conditions that can be changed during the lifetime of the
>       subscription.";
>    choice update-trigger {
>      description
>        "Defines necessary conditions for sending an event record to
>         the subscriber.";
>      case periodic {
>        container periodic {
>          presence "indicates a periodic subscription";
>          description
>            "The publisher is requested to notify periodically the
>             current values of the datastore as defined by the
>             selection filter.";
>          leaf period {
>            type yang:timeticks;
>            mandatory true;
>            description
>              "Duration of time which should occur between periodic
>               push updates, in one hundredths of a second.";
>          }
>          leaf anchor-time {
>            type yang:date-and-time;
>            description
>              "Designates a timestamp before or after which a series
>               of periodic push updates are determined. The next
>               update will take place at a whole multiple interval
>               from the anchor time.  For example, for an anchor time
>               is set for the top of a particular minute and a period
>               interval of a minute, updates will be sent at the top
>               of every minute this subscription is active.";
>          }
>        }
>      }
>      case on-change {
>        if-feature "on-change";
>        container on-change {
>          presence "indicates an on-change subscription";
>          description
>            "The publisher is requested to notify changes in values
>             in the datastore subset as defined by a selection
>             filter.";
>          leaf dampening-period {
>            type yang:timeticks;
>            default "0";
>            description
>              "Specifies the minimum interval between the assembly of
>               successive update records for a single receiver of a
>               subscription.  Whenever subscribed objects change, and
>               a dampening period interval (which may be zero) has
>               elapsed since the previous update record creation for
>               a receiver, then any subscribed objects and properties
>               which have changed since the previous update record
>               will have their current values marshalled and placed
>               into a new update record.";
>          }
>        }
>      }
>    }
>  }
> 
>  grouping update-policy {
>    description
>      "This grouping describes the datastore-specific subscription
>       conditions of a subscription.";
>    uses update-policy-modifiable {
>      augment "update-trigger/on-change/on-change" {
>        description
>          "Includes objects not modifiable once subscription is
>           established.";
>        leaf sync-on-start {
>          type boolean;
>          default "true";
>          description
>            "When this object is set to false, it restricts an
>             on-change subscription from sending push-update
>             notifications.  When false, pushing a full selection per
>             the terms of the selection filter MUST NOT be done for
>             this subscription.  Only updates about changes,
>             i.e. only push-change-update notifications are sent.
>             When true (default behavior), in order to facilitate a
>             receiver's synchronization, a full update is sent when
>             the subscription starts using a push-update
>             notification.  After that, push-change-update
>             notifications are exclusively sent unless the publisher
>             chooses to resync the subscription via a new push-update
>             notification.";
>        }
>        leaf-list excluded-change {
>          type change-type;
>          description
>            "Use to restrict which changes trigger an update.  For
>             example, if modify is excluded, only creation and
>             deletion of objects is reported.";
>        }
>      }
>    }
>  }
> 
>  grouping hints {
>    description
>      "Parameters associated with some error for a subscription
>       made upon a datastore.";
>    leaf period-hint {
>      type yang:timeticks;
>      description
>        "Returned when the requested time period is too short. This
>         hint can assert a viable period for either a periodic push
>         cadence or an on-change dampening interval.";
>    }
>    leaf filter-failure-hint {
>      type string;
>      description
>        "Information describing where and/or why a provided filter
>         was unsupportable for a subscription.";
>    }
>    leaf object-count-estimate {
>      type uint32;
>      description
>        "If there are too many objects which could potentially be
>         returned by the selection filter, this identifies the
>         estimate of the number of objects which the filter would
>         potentially pass.";
>    }
>    leaf object-count-limit {
>      type uint32;
>      description
>        "If there are too many objects which could be returned by
>         the selection filter, this identifies the upper limit of
>         the publisher's ability to service for this subscription.";
>    }
>    leaf kilobytes-estimate {
>      type uint32;
>      description
>        "If the returned information could be beyond the capacity
>         of the publisher, this would identify the data size which
>         could result from this selection filter.";
>    }
>    leaf kilobytes-limit {
>      type uint32;
>      description
>        "If the returned information would be beyond the capacity
>         of the publisher, this identifies the upper limit of the
>         publisher's ability to service for this subscription.";
>    }
>  }
> 
>  /*
>   * RPCs
>   */
> 
>  rpc resync-subscription {
>    if-feature "on-change";
>    description
>      "This RPC allows a subscriber of an active on-change
>       subscription to request a full push of objects.
> 
>       A successful invocation results in a push-update of all
>       datastore nodes that the subscriber is permitted to access.
>       This RPC can only be invoked on the same session on which the
>       subscription is currently active.  In case of an error, a
>       resync-subscription-error is sent as part of an error
>       response.";
>    input {
>      leaf id {
>        type sn:subscription-id;
>        mandatory true;
>        description
>          "Identifier of the subscription that is to be resynced.";
>      }
>    }
>  }
> 
>  rc:yang-data resync-subscription-error {
>    container resync-subscription-error {
>      description
>        "If a 'resync-subscription' RPC fails, the subscription is
>         not resynced and the RPC error response MUST indicate the
>         reason for this failure.  This yang-data MAY be inserted as
>         structured data within a subscription's RPC error response
>         to indicate the failure reason.";
>      leaf reason {
>        type identityref {
>          base resync-subscription-error;
>        }
>        mandatory true;
>        description
>          "Indicates the reason why the publisher has declined a
>           request for subscription resynchronization.";
>      }
>      uses hints;
>    }
>  }
> 
>  augment "/sn:establish-subscription/sn:input" {
>    description
>      "This augmentation adds additional subscription parameters
>       that apply specifically to datastore updates to RPC input.";
>    uses update-policy;
>  }
> 
>  augment "/sn:establish-subscription/sn:input/sn:target" {
>    description
>      "This augmentation adds the datastore as a valid target
>       for the subscription to RPC input.";
>    case datastore {
>      description
>        "Information specifying the parameters of an request for a
>         datastore subscription.";
>      uses datastore-criteria;
>    }
>  }
> 
>  rc:yang-data establish-subscription-datastore-error-info {
>    container establish-subscription-datastore-error-info {
>      description
>        "If any 'establish-subscription' RPC parameters are
>         unsupportable against the datastore, a subscription is not
>         created and the RPC error response MUST indicate the reason
>         why the subscription failed to be created.  This yang-data
>         MAY be inserted as structured data within a subscription's
>         RPC error response to indicate the failure reason.  This
>         yang-data MUST be inserted if hints are to be provided back
>         to the subscriber.";
>      leaf reason {
>        type identityref {
>          base sn:establish-subscription-error;
>        }
>        description
>          "Indicates the reason why the subscription has failed to
>           be created to a targeted datastore.";
>      }
>      uses hints;
>    }
>  }
> 
>  augment "/sn:modify-subscription/sn:input" {
>    description
>      "This augmentation adds additional subscription parameters
>       specific to datastore updates.";
>    uses update-policy-modifiable;
>  }
> 
>  augment "/sn:modify-subscription/sn:input/sn:target" {
>    description
>      "This augmentation adds the datastore as a valid target
>       for the subscription to RPC input.";
>    case datastore {
>      description
>        "Information specifying the parameters of an request for a
>         datastore subscription.";
>      uses datastore-criteria;
>    }
>  }
> 
>  rc:yang-data modify-subscription-datastore-error-info {
>    container modify-subscription-datastore-error-info {
>      description
>        "This yang-data MAY be provided as part of a subscription's
>         RPC error response when there is a failure of a
>         'modify-subscription' RPC which has been made against a
>         datastore.  This yang-data MUST be used if hints are to be
>         provides back to the subscriber.";
>      leaf reason {
>        type identityref {
>          base sn:modify-subscription-error;
>        }
>        description
>          "Indicates the reason why the subscription has failed to
>           be modified.";
>      }
>      uses hints;
>    }
>  }
> 
>  /*
>   * NOTIFICATIONS
>   */
> 
>  notification push-update {
>    description
>      "This notification contains a push update, containing data
>       subscribed to via a subscription.  This notification is sent
>       for periodic updates, for a periodic subscription.  It can
>       also be used for synchronization updates of an on-change
>       subscription.  This notification shall only be sent to
>       receivers of a subscription.  It does not constitute a
>       general-purpose notification that would be subscribable as
>       part of the NETCONF event stream by any receiver.";
>    leaf id {
>      type sn:subscription-id;
>      description
>        "This references the subscription which drove the
>         notification to be sent.";
>    }
>    anydata datastore-contents {
>      description
>        "This contains the updated data.  It constitutes a snapshot
>         at the time-of-update of the set of data that has been
>         subscribed to.  The snapshot corresponds to the same
>         snapshot that would be returned in a corresponding get
>         operation with the same selection filter parameters
>         applied.";
>    }
>    leaf incomplete-update {
>      type empty;
>      description
>        "This is a flag which indicates that not all datastore
>         nodes subscribed to are included with this update.  In
>         other words, the publisher has failed to fulfill its full
>         subscription obligations, and despite its best efforts is
>         providing an incomplete set of objects.";
>    }
>  }
> 
>  notification push-change-update {
>    if-feature "on-change";
>    description
>      "This notification contains an on-change push update. This
>       notification shall only be sent to the receivers of a
>       subscription; it does not constitute a general-purpose
>       notification.";
>    leaf id {
>      type sn:subscription-id;
>      description
>        "This references the subscription which drove the
>         notification to be sent.";
>    }
>    container datastore-changes {
>      description
>        "This contains the set of datastore changes of the target
>         datastore starting at the time of the previous update, per
>         the terms of the subscription.  The datastore changes are
>         encoded per RFC 8027 (YANG Patch).";
>      uses ypatch:yang-patch;
>    }
>    leaf incomplete-update {
>      type empty;
>      description
>        "The presence of this object indicates not all changes which
>         have occurred since the last update are included with this
>         update.  In other words, the publisher has failed to
>         fulfill its full subscription obligations, for example in
>         cases where it was not able to keep up with a change
>         burst.";
>    }
>  }
> 
>  augment "/sn:subscription-started" {
>    description
>      "This augmentation adds datastore-specific objects to
>       the notification that a subscription has started.";
>    uses update-policy;
>  }
> 
>  augment "/sn:subscription-started/sn:target" {
>    description
>      "This augmentation allows the datastore to be included as
>       part of the notification that a subscription has started.";
>    case datastore {
>      uses datastore-criteria {
>        refine "selection-filter/within-subscription" {
>          description
>            "Specifies the selection filter and where it originated
>             from.  If the 'selection-filter-ref' is populated, the
>             filter within the subscription came from the 'filters'
>             container.  Otherwise it is populated in-line as part of
>             the subscription itself.";
>        }
>      }
>    }
>  }
> 
>  augment "/sn:subscription-modified" {
>    description
>      "This augmentation adds datastore-specific objects to
>       the notification that a subscription has been modified.";
>    uses update-policy;
>  }
> 
>  augment "/sn:subscription-modified/sn:target" {
>    description
>      "This augmentation allows the datastore to be included as
>       part of the notification that a subscription has been
>       modified.";
>    case datastore {
>      uses datastore-criteria {
>        refine "selection-filter/within-subscription" {
>          description
>            "Specifies where the selection filter, and where it came
>             from within the subscription and then populated within
>             this notification.  If the 'selection-filter-ref' is
>             populated, the filter within the subscription came from
>             the 'filters' container.  Otherwise it is populated
>             in-line as part of the subscription itself.";
>        }
>      }
>    }
>  }
> 
>  /*
>   * DATA NODES
>   */
> 
>  augment "/sn:filters" {
>    description
>      "This augmentation allows the datastore to be included as part
>       of the selection filtering criteria for a subscription.";
>    list selection-filter {
>      key "filter-id";
>      description
>        "A list of pre-configured filters that can be applied
>         to datastore subscriptions.";
>      leaf filter-id {
>        type string;
>        description
>          "An identifier to differentiate between selection
>           filters.";
>      }
>      uses selection-filter-types;
>    }
>  }
> 
>  augment "/sn:subscriptions/sn:subscription" {
>    when 'yp:datastore';
>    description
>      "This augmentation adds many datastore specific objects to a
>       subscription.";
>    uses update-policy;
>  }
> 
>  augment "/sn:subscriptions/sn:subscription/sn:target" {
>    description
>      "This augmentation allows the datastore to be included as
>       part of the selection filtering criteria for a subscription.";
>    case datastore {
>      uses datastore-criteria;
>    }
>  }
> }
> _______________________________________________
> netconf mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/netconf

---

From: "Alexander Clemm" <[email-masked]>
Date: Mon, 4 Feb 2019 22:39:43 -0800

Hi Martin,

Thank you for your review!  We have just posted an updated revision (-22)
per your comments.  (Please see replies inline, <ALEX>)

--- Alex

-----Original Message-----
From: netconf <[email-masked]> On Behalf Of Martin Bjorklund
Sent: Wednesday, January 30, 2019 7:51 AM
To: [email-masked]; [email-masked]
Cc: [email-masked]
Subject: [netconf] Yangdoctors last call review of
draft-ietf-netconf-yang-push

Hi,

o  General

  <ALEX> Done </ALEX>

o  General

       (requires pyang 1.7.8)

  <ALEX> Thank you </ALEX>

o  Figure 6

  OLD:

     notifications:
       +---n push-update

  NEW:

     notifications:
       +---n push-update

<ALEX> Updated </ALEX>
          
o  Figure 6

  <ALEX>  I am splitting the tree into smaller pieces as requested, although
I am not sure this is really needed.  While I agree with you in principle,
the tree output is structured a bit (into rpcs, notifications, etc), and has
items that are non-applicable dotted out  ("...").  It is simply a big
module, I guess...
  </ALEX>

o  Figure 9

  OLD:

  NEW:

   <rpc message-id="101"

<ALEX> Done </ALEX>
o  Figure 11

  Similar in this example:

  OLD:

  NEW:

   <rpc message-id="102"

<ALEX> Done </ALEX>

o  Section 4.4.4

  OLD:

  NEW:

   <rpc message-id="103"

<ALEX> Done (no idea why no number is being generated for the number)
</ALEX>

o  Section 5.

<ALEX> Done </ALEX> 
   
o  import yang-patch

  OLD:

    reference
      "RFC 8072: YANG Patch";

  NEW:

    reference

      <ALEX> Done </ALEX> 

o  typedef change-type

  I suggest:

  OLD:

    description

   NEW:

    description

<ALEX> Done </ALEX> 

    type centiseconds {
      type uint32;
      description

  and use that instead.

  <ALEX> Done (added typedef centiseconds and updated the diagrams
accordingly)</ALEX> 

  OLD:

  NEW:

   <ALEX> Done </ALEX> 

  OLD:

  NEW:

<ALEX> Done </ALEX>

o  Section A.1

<ALEX> Done.  (s/within the either the/in the/ and spelled out the names of
the YANG modules) <ALEX> 
  
o  Section A.1

  So I suggest:

  OLD:

  NEW:

<ALEX>Done, and changed the introductory paragraph accordingly.  </ALEX>  

o  Section A.2

<ALEX> Done, with corresponding changes to the introductory paragraph.
</ALEX> 

/martin

---

From: Martin Bjorklund <[email-masked]>
Date: Tue, 05 Feb 2019 09:03:12 +0100 (CET)

Hi,

> per your comments.  (Please see replies inline, <ALEX>)

Thanks!  I have reviewed the changes, and the new version addresses
all my comments.

FYI, I found a few nits that you can fix if you post a new version, or
during AUTH48:

OLD:

           Figure 9: Model structure (non-augmentation portions

NEW:

           Figure 9: Model structure (non-augmentation portions)

And some formatting issues in the new text added to the YANG module.
You can reach out to me directly if you want these, or just run pyang
yourself.

/martin

---

From: Alexander Clemm <[email-masked]>
Date: Tue, 5 Feb 2019 17:59:49 +0000

Thanks, Martin!
--- Alex

> From: netconf [mailto:[email-masked]] On Behalf Of Martin
> Bjorklund
> Sent: Tuesday, February 05, 2019 12:03 AM
> To: [email-masked]
> Cc: [email-masked]; [email-masked];
> [email-masked]
> Subject: Re: [netconf] Yangdoctors last call review of draft-ietf-netconf-yang-
> push
> 
> Hi,
> 
> 
> > Thank you for your review!  We have just posted an updated revision
> > (-22) per your comments.  (Please see replies inline, <ALEX>)
> 
> Thanks!  I have reviewed the changes, and the new version addresses all my
> comments.
> 
> FYI, I found a few nits that you can fix if you post a new version, or during
> AUTH48:
> 
> 
> OLD:
> 
> You can reach out to me directly if you want these, or just run pyang yourself.
> 
> 
> 
> /martin
> 
> 
> 
> 
> >
> > --- Alex
> >
> >   <ALEX>  I am splitting the tree into smaller pieces as requested,
> > although I am not sure this is really needed.  While I agree with you
> > in principle, the tree output is structured a bit (into rpcs,
> > notifications, etc), and has items that are non-applicable dotted out
> > ("...").  It is simply a big module, I guess...
> >   </ALEX>
> >
> > o  Figure 9
> >
> > <ALEX> Done.  (s/within the either the/in the/ and spelled out the
> > names of the YANG modules) <ALEX>
> >
> > o  Section A.1
> >