Re: [yang-doctors] [netmod] Yangdoctors last call review of draft-ietf-netmod-nmda-diff-04

[Alexander L Clemm <[e-mail]>]

Thank you! 

I just uploaded rev -06.

--- Alex

On 9/18/2020 12:47 PM, Reshad Rahman (rrahman) wrote:
>
> Hi Alex,
>
>  
>
> This addresses my comment/concern.
>
>  
>
> Regards,
>
> Reshad.
>
>  
>
> *From: *Alexander L Clemm <[e-mail]>
> *Date: *Friday, September 18, 2020 at 3:43 PM
> *To: *"Reshad Rahman (rrahman)" <[e-mail]>,
> "[e-mail]" <[e-mail]>
> *Cc: *"[e-mail]" <[e-mail]>, "[e-mail]"
> <[e-mail]>, "[e-mail]"
> <[e-mail]>
> *Subject: *Re: [yang-doctors] [netmod] Yangdoctors last call review of
> draft-ietf-netmod-nmda-diff-04
>
>  
>
> Hi Reshad,
>
> okay, so let's add the following then to section 4, in the explanation
> of the "differences" output parameter:
>
> "When a datastore node in the source of the comparison is not present
> in the target of the comparison, this can be indicated either as a
> "delete" or as a "remove" in the patch as there is no differentiation
> between those operations for the purposes of the comparison.  "
>
> And update the description as follows:
>
>          container differences {
>           description
>            "The list of differences, encoded per RFC8072 with an
>              augmentation to include source values where
>              applicable.  When a datastore node in the source is
>              not present in the target, this can be indicated either
>              as a 'delete' or as a 'remove' as there is no difference
>              between them for the purposes of the comparison.";
> ...
>
> I will post this in a -06 shortly.  Please let us know if this
> addresses your concerns or if there is anything else.
>
> Thanks!
>
> --- Alex
>
>  
>
> On 9/18/2020 5:57 AM, Reshad Rahman (rrahman) wrote:
>
>     Hi Alex,
>
>      
>
>     I think the only “problem” with using both “remove” and “delete”
>     is that it could be confusing (when should one be used and not the
>     other). Adding some text to say they’re the same for the diff
>     operation is good enough for me.
>
>      
>
>     Regards,
>
>     Reshad.
>
>      
>
>     *From: *Alexander L Clemm <[e-mail]> <mailto:[e-mail]>
>     *Date: *Tuesday, September 15, 2020 at 7:31 PM
>     *To: *"Reshad Rahman (rrahman)" <[e-mail]>
>     <mailto:[e-mail]>, "[e-mail]"
>     <mailto:[e-mail]> <[e-mail]>
>     <mailto:[e-mail]>
>     *Cc: *"[e-mail]" <mailto:[e-mail]>
>     <[e-mail]> <mailto:[e-mail]>,
>     "[e-mail]" <mailto:[e-mail]> <[e-mail]>
>     <mailto:[e-mail]>,
>     "[e-mail]"
>     <mailto:[e-mail]>
>     <[e-mail]>
>     <mailto:[e-mail]>
>     *Subject: *Re: [yang-doctors] [netmod] Yangdoctors last call
>     review of draft-ietf-netmod-nmda-diff-04
>
>      
>
>     Hi Reshad,
>
>     re: question 1: As you indicate, there may be no distinction
>     between indicating a "remove" or a "delete" in the patch.  Right
>     now it would be acceptable to return either.  If we want to
>     eliminate this freedom, which one would you prefer be used?  Shall
>     we remove the possibility for "delete" and just cover it using
>     "remove"? 
>
>     Note that the place where this is specified in the model is as
>     part of a condition that specifies when the source value should be
>     included.   If we want to rule out that diff can return either
>     "remove" or "delete" (indeed they are synonymous), we would need
>     to add text to the container description that when a data object
>     is present in the target of the comparison but not the source,
>     that "remove" should be used to indicate that.
>
>     The model would be changed follows.  Please confirm if this looks
>     good to you & we'll incorporate it. 
>
>     OLD
>
>                container differences {
>
>                  description
>
>                    "The list of differences, encoded per RFC8072
>     <https://tools.ietf.org/html/rfc8072> with an
>
>                     augmentation to include source values where
>
>                     applicable.";
>
>                  uses ypatch:yang-patch {
>
>                    augment "yang-patch/edit" {
>
>                      description
>
>                        "Provide the value of the source of the patch,
>
>                         respectively of the comparison, in addition to
>
>                         the target value, where applicable.";
>
>                      anydata source-value {
>
>                        when "../operation = 'delete'"
>
>                          + "or ../operation = 'merge'"
>
>                          + "or ../operation = 'move'"
>
>                          + "or ../operation = 'replace'"
>
>                          + "or ../operation = 'remove'";
>
>                        description
>
>                          "The anydata 'value' is only used for 'delete',
>
>                           'move', 'merge', 'replace', and 'remove'
>
>                           operations.";
>
>                      }
>
>                      reference "RFC 8072
>     <https://tools.ietf.org/html/rfc8072>: YANG Patch Media Type";
>
>                    }
>
>                  }
>
>                }
>
>      
>
>      
>
>     NEW:
>
>                container differences {
>
>                  description
>
>                    "The list of differences, encoded per RFC8072
>     <https://tools.ietf.org/html/rfc8072> with an
>
>                     augmentation to include source values where
>
>                     applicable.  Where a difference include a data object
>
>                     in the target that is not present in the source,
>
>                     this shall be indicated as a 'remove' operation
>
>                     in the patch, not as a 'delete' operation.";
>
>                  uses ypatch:yang-patch {
>
>                    augment "yang-patch/edit" {
>
>                      description
>
>                        "Provide the value of the source of the patch,
>
>                         respectively of the comparison, in addition to
>
>                         the target value, where applicable.";
>
>                      anydata source-value {
>
>                        when "../operation = 'merge'"
>
>                          + "or ../operation = 'move'"
>
>                          + "or ../operation = 'replace'"
>
>                          + "or ../operation = 'remove'";
>
>                        description
>
>                          "The anydata 'value' is only used for 'merge',
>
>                           'move','replace', and 'remove' operations.";
>
>                      }
>
>                      reference "RFC 8072
>     <https://tools.ietf.org/html/rfc8072>: YANG Patch Media Type";
>
>                    }
>
>                  }
>
>                }
>
>      
>
>     Thanks
>
>     --- Alex
>
>      
>
>     On 9/15/2020 4:04 PM, Reshad Rahman (rrahman) wrote:
>
>         Hi Alex,
>
>          
>
>         I will review the latest version.
>
>          
>
>         See below for questions/responses.
>
>          
>
>         On 2020-09-15, 5:19 PM, "yang-doctors on behalf of Alexander L Clemm" <[e-mail] on behalf of [e-mail]> <mailto:[e-mail]@clemm.org> wrote:
>
>          
>
>             Hello Reshad, hello YANG Doctors,
>
>          
>
>             thank you for your review!  Please find my replies inline, <ALEX>.  We
>
>             have also just posted -05 (thanks, Yingzhen, for doublechecking my
>
>             updates).   
>
>          
>
>             --- Alex on behalf of coauthors
>
>          
>
>             On 9/7/2020 7:06 AM, Reshad Rahman (rrahman) wrote:
>
>             > <Here's the same message with hopefully more readable formatting>
>
>             >
>
>             > Review of rev -04 by Reshad Rahman
>
>             >
>
>             > The document is clear and well-written. While some issues have been identified, they can be resolved quickly.
>
>             >
>
>         <snip>
>
>          
>
>             > Questions
>
>             >    1.      YANG model: does the operation “delete” make sense for a diff operation? If it is kept, it’d be good to have some text explaining that for a diff operation, “delete” and “replace” are the same? If they’re not the same, please also add some text….
>
>         <RR> I actually meant "delete" and "remove".
>
>             <ALEX> Here we are simply referring to the basic YANG-patch edit
>
>             operations per https://tools.ietf.org/html/rfc8072#page-11.  Those are
>
>             in turn derived from <edit-config> operations per
>
>             https://tools.ietf.org/html/rfc6241#page-37.  I am not sure we need add
>
>             to explain those, as we are directly referring to YANG-patch. 
>
>          
>
>             </ALEX>
>
>         <RR> The operations are indeed well defined in RFC8072 (copied below), but they are defined from the perspective of YANG-Patch. So for YANG-Patch "delete" and "remove" are different operations, but from a diff comparison I believe they are the same (the resource must exist since it's being returned in a diff)
>
>          
>
>            +-----------+-----------------------------------------------------------------+
>
>            | delete    | delete a data resource if it already exists; if it    |
>
>            |                | does not exist, return an error                               |
>
>            |                |                                                                                      |
>
>            | remove | remove a data resource if it already exists           |
>
>            +-----------+-----------------------------------------------------------------+
>
>          
>
>             >    3.      YANG model P9, for the “uses path:yang-patch”, why not have a  reference to RFC8072 (is it because the description above mentions RFC8072)?
>
>             <ALEX> We are clearly referencing RFC 8072; are you suggesting to put a
>
>             reference substatement below the uses statement?   It looks a little
>
>             strange to me but sure, we will add it.   
>
>         <RR> Not needed. 
>
>          
>
>             >    4.      Section 7 mentions rate limiting requests per client. Should there be a “global” rate-limiting too, i.e not client-specific?
>
>          
>
>             <ALEX> I am not sure this is really needed as I think the number of
>
>             management clients will in general be fairly limited to begin with, but
>
>             we can certainly add it.  How about the following text:
>
>          
>
>             OLD:
>
>          
>
>             One possibility for an implementation to mitigate against such a
>
>             possibility is to limit the number of requests that is served to a
>
>             client in any one time interval, rejecting requests made at a higher
>
>             frequency than the implementation can reasonably sustain.
>
>          
>
>             NEW:
>
>          
>
>             One possibility for an implementation to mitigate against such a
>
>             possibility is to limit the number of requests that is served to a
>
>             client, or to any number of clients, in any one time interval, rejecting
>
>             requests made at a higher frequency than the implementation can
>
>             reasonably sustain.
>
>         <RR> Good with me.
>
>          
>
>             </ALEX>
>
>          
>
>             >    5.      Wondering if section 8 should be in an Appendix (or even removed)? Also, the method suggested doesn’t seem to guarantee that the difference persisted for the “dampening” time.
>
>          
>
>             <ALEX> Personally, I do think it makes sense to include a brief
>
>             discussion of possible further extensions.  I suggest to keep the
>
>             section if it's okay with you, or perhaps leave it to the chair whether
>
>             they have a preference to remove it.  
>
>          
>
>             </ALEX>
>
>         <RR>Whatever the WG/chairs decide is fine with me.
>
>          
>
>         Regards,
>
>         Reshad.
>
>          
>
>          
>