Re: [Netconf] Last Call on yang-push-17

["Reshad Rahman (rrahman)" <[e-mail]>]

Hi Alex,

Good on all fronts.

Regards,
Reshad.


On 2018-08-28, 8:23 PM, "Alexander Clemm" <[e-mail]> wrote:

    Hi Reshad,
    
    thank you for your comments!
    
    Please find responses inline, <ALEX>
    
    --- Alex
    
    > -----Original Message-----
    > From: Netconf [mailto:[e-mail]] On Behalf Of Reshad
    > Rahman (rrahman)
    > Sent: Monday, August 27, 2018 7:18 PM
    > To: Kent Watsen <[e-mail]>; [e-mail]
    > Subject: Re: [Netconf] Last Call on yang-push-17
    > 
    > Hi,
    > 
    > I believe yang-push-17 is in very good shape and is ready to progress to the
    > next step. Here are my nits/comments/questions, nothing major:
    > 
    > - Both "rpc" and 'RPC" are used, should be "RPC" everywhere?
    
    <ALEX> updated
    </ALEX>
    
    > - Abstract mentions "remote mirroring", should we really assume that these
    > new capabilities all depend on "remote mirroring" of state?
    
    <ALEX> The text states that this "enables new capabilities based on the remote mirroring of configuration and operational state".  There may be other capabilities as well.  From that perspective I don't think this is controversial.  That said, you are the second person who takes issue with remote mirroring, which makes me wonder if the sentence should be rephrased.  How about: " Providing such visibility into updates enables new capabilities based on the remote mirroring and monitoring of configuration and operational state."
    </ALEX>
    
    > - In Section 2 "Definitions and acronyms", there's mention that the
    > terminology defined in RFCs  7950, 8341 and 8342 is used. We should also
    > mention the subscribed-notifications draft since terminology such as
    > subscribers, receivers etc is defined in that draft.
    
    <ALEX> Thank you, updated
    </ALEX>
    
    > - Section 2 last bullet should be "The subscription and push mechanisms for
    > datastore updates that are specified..."
    
    <ALEX> I believe that "is" is correct, because it refers to the mechanism, not to the updates (and it is one mechanism, not multiple) </ALEX>
    
    > - Section 3 should be "a solution that provides a subscription service"
    
    <ALEX> thank you, updated </ALEX>
    
    > - Section 3.1 "additional parameters such as" implies there are more
    > parameters. I believe there aren't more parameters? If there, add a a
    > reference to the proper section(s)?
    
    <ALEX> changed "such as" to "that include" </ALEX>
    
    > - Section 3.1 "... to exhaust of resources" should be "to exhaust resources"
    
    <ALEX> noted, changed </ALEX>
    
    > - Section 3.1 "specify the interval which must pass before successive",
    > change "must" to "MUST"?
    
    <ALEX> I think "must" is correct, since we cannot impose requirements on an interval.  However, to avoid ambiguities, changing "which must pass" to "which has to pass" </ALEX>
    
    > - Section 3.1 (Page6), change "you might only send when an object is created
    > or deleted" to "the publisher might only send notifications when an object is
    > created or deleted"?
    
    <ALEX> Changed to "might only send an update when ..." </ALEX>
    
    > - Section 3.3 first sentence "allow subscribers to receive updates" should be
    > "allow receivers to receive updates"?
    
    <ALEX> changed </ALEX>
    
    > - Section 3.3. bullet 3 for YANG patch record, add reference ro RFC8072?
    
    <ALEX> ok </ALEX>
    
    > - Section 3.4 2nd paragraph "publisher notifies receivers immediately and
    > reliably whenever...", is it the receivers which are notified in such a situation
    > or is it the subscriber?
    
    <ALEX> It is the receiver, as is stated in the text </ALEX>
    
    > - Section 3.5.2 s/datstore/datastore/
    
    <ALEX> corrected </ALEX>
    
    > - Section 3.5.2 2nd paragraph, text uses past tense "was created", "was
    > deleted", should the present be used instead?
    
    <ALEX> I think "past" is fine.  This concerns that an update has taken place.  There may be dampening which causes updates to reported that have already occurred in the past.  </ALEX>
    
    > - Section 3.5.2 3rd paragraph, "However a patch must be able...", should that
    > say "patch record"?
    
    <ALEX> We call it "patch" throughout this section.  I think it is clear what is meant here.  </ALEX>
    
    > - Section 3.5.2 bottom of Page 9, "YANG push" should be changed to "YANG-
    > Push".
    
    <ALEX> ok </ALEX>
    
    > - Section 3.6 last sentence mentions "push-update" and "push-change-
    > update", add reference to section 3.7?
    
    <ALEX> ok </ALEX>
    
    > - Section 3.7 4th paragraph, sentence "These new "push-update" and "push-
    > change-update" are encoded..." doesn't read well. But I'm not sure how to
    > make it better, maybe add "events" before "are encoded"?
    
    <ALEX> rephrased as follows: " "Push-update" and "push-change-update" are encoded and placed within notification messages, and ultimately queued for egress over the specified transport. " </ALEX>
    
    > - Section 3.8 3rd paragraph when "establish-subscription-stream-error-info"
    > is mentioned, add a reference?
    
    <ALEX> Don't think this is needed.  This obviously discusses the data model, where this is defined.  </ALEX>
    
    > - Section 3.9 first sentence should finish with "it has proper authorization"
    > (receiver is singular).
    
    <ALEX> ok </ALEX>
    
    > - Section 3.9 3rd paragraph first sentence should be "A publisher MAY choose
    > to reject...."
    
    <ALEX> ok </ALEX>
    
    > - Section 3.10, first sentence change the last part to "to push on-change
    > updates for some object types"?
    
    <ALEX> ok </ALEX> 
    
    > - Section 4.4.1 Figure 10. There seems to be a mistake in the XML example for
    > the error response, we have </error-message> instead of </error-app-tag>
    
    <ALEX> Example has been corrected per Martin's comment </ALEX> 
    
    > - YANG model P37 presence "indicates an periodic subscription", s/an/a/
    
    <ALEX> ok </ALEX>
     
    > - YANG model P38, fix indentation on 2nd line of description
    
    <ALEX> ok </ALEX>
    
    > - YANG model P39, leaf nodes kilobytes-limit and kilobytes-estimate, why
    > not use units "kilobyte" and rename these 2 leaf nodes to limit and estimate?
    
    <ALEX> well, the name indicates the unit, guess it's a matter of taste, leaving it unchanged (unless you feel strongly about it). 
    Thanks again for your comments!
    --- Alex
     </ALEX> 
    
    > 
    > Regards,
    > Reshad.
    > 
    > On 2018-08-14, 1:28 PM, "Netconf on behalf of Kent Watsen" <netconf-
    > [e-mail] on behalf of [e-mail]> wrote:
    > 
    >     This message starts a Last Call on draft-ietf-netconf-yang-push-17:
    > 
    >       https://tools.ietf.org/html/draft-ietf-netconf-yang-push-17
    > 
    > 
    >     This marks the beginning of the last calls on the yang push suite of drafts.
    >     Given the size and number of documents, the chairs decided to break the
    >     reviews up into pieces so as to get focus on each in turn.  We are choosing
    >     to go top-down, starting with yang-push and ending with the "notif" drafts.
    >     We plan to submit the drafts for publication when they are ready as a
    >     collective.  The goal is to do all this prior to IETF 103.
    > 
    >     We understand that, in reviewing yang-push, there is a need to consider
    > the
    >     subscribed-notifications draft.  We will not be surprised if, in the course
    >     of things, both drafts are updated, even though the review is primarily on
    >     the yang-push draft.
    > 
    >     While it's always nice to receive messages of support, at this time, the
    >     question isn't so much if the working group supports the work, than if
    >     the document is ready to progress.  The chairs need to see reviews that
    >     indicate thorough end-to-end reading of the text.  Of course, if there
    >     are any objections, these should be brought forward now as well.
    > 
    >     The current version (-17) of this draft was published on July 1st, just
    >     before the IETF 102 meeting.  The datatracker page for the draft is here:
    >     https://datatracker.ietf.org/doc/draft-ietf-netconf-yang-push.
    > 
    > 
    >     Thanks,
    >     Kent (and Mahesh)
    > 
    > 
    >     _______________________________________________
    >     Netconf mailing list
    >     [e-mail]
    >     https://www.ietf.org/mailman/listinfo/netconf
    > 
    > 
    > _______________________________________________
    > Netconf mailing list
    > [e-mail]
    > https://www.ietf.org/mailman/listinfo/netconf