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

Alexander L Clemm <ludwig@clemm.org> Fri, 18 September 2020 19:42 UTC

Return-Path: <ludwig@clemm.org>
X-Original-To: netmod@ietfa.amsl.com
Delivered-To: netmod@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 9F0EC3A0CF4; Fri, 18 Sep 2020 12:42:31 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.918
X-Spam-Level:
X-Spam-Status: No, score=-1.918 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, HTML_MESSAGE=0.001, NICE_REPLY_A=-0.001, RCVD_IN_MSPIKE_H3=-0.01, RCVD_IN_MSPIKE_WL=-0.01, SPF_NONE=0.001, URIBL_BLOCKED=0.001] autolearn=ham autolearn_force=no
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id wQoGAkWF3Tba; Fri, 18 Sep 2020 12:42:29 -0700 (PDT)
Received: from mout.perfora.net (mout.perfora.net [74.208.4.194]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id D72073A07F2; Fri, 18 Sep 2020 12:42:28 -0700 (PDT)
Received: from [172.16.0.44] ([73.189.160.186]) by mrelay.perfora.net (mreueus004 [74.208.5.2]) with ESMTPSA (Nemesis) id 1MTisb-1jtsGV3Pjx-00TzFI; Fri, 18 Sep 2020 21:42:27 +0200
To: "Reshad Rahman (rrahman)" <rrahman@cisco.com>, "yang-doctors@ietf.org" <yang-doctors@ietf.org>
Cc: "last-call@ietf.org" <last-call@ietf.org>, "netmod@ietf.org" <netmod@ietf.org>, "draft-ietf-netmod-nmda-diff.all@ietf.org" <draft-ietf-netmod-nmda-diff.all@ietf.org>
References: <159942490640.25028.10946254095755778899@ietfa.amsl.com> <EF21460A-8689-491C-AE19-942C6FA84FFC@cisco.com> <e801c95e-078e-8438-b1a0-18aaf4be3a82@clemm.org> <8759A9BF-300C-46F7-B39F-9EF4CFA2D726@cisco.com> <22126972-0920-1bb3-a73f-f4a219a59bf6@clemm.org> <0E3A16A2-6ABA-4868-936F-AA6C9AAF3A8E@cisco.com>
From: Alexander L Clemm <ludwig@clemm.org>
Message-ID: <7cf5120e-28c9-383a-5238-0d6749e93854@clemm.org>
Date: Fri, 18 Sep 2020 12:42:25 -0700
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.12.0
MIME-Version: 1.0
In-Reply-To: <0E3A16A2-6ABA-4868-936F-AA6C9AAF3A8E@cisco.com>
Content-Type: multipart/alternative; boundary="------------A848C0F38A0E06E879280179"
Content-Language: en-US
X-Provags-ID: V03:K1:fhThEQlhJG0oxwtk+pijyByX2lucLGR6xaBX0QV/SN41YOq+T6j raoV7U8Jo/Y8PUvkh8do0MCNll/rx+7V4fmkifnoNb+0J7qKVPrZn9DJGqyMZfiifDa1K43 DB3MallC4mVh+PrvmLLbAWiM01JaLyfoTNe1Zk7d8Q0XnUpVdDmAw0iMcZDJABp6VH0DT1Z TGEuy0RoRAUwW1gRCFotQ==
X-UI-Out-Filterresults: notjunk:1;V03:K0:47vvpk98Qtw=:tQY60kVRhxR81m3llUf1WZ Yid1OL0zHY2tlawM7R2pye5Ji2IAQt1+NP+wshiN460wdiUTCJj0yta2i3pH1hZCXaQnno4xC vf1kCfc1fUOiCrnHDqCx2BeT1M30RSXyTRoaUW9mn2DBw/kMq5FqYgRjyojp0A0LzT3rn0pns 4olLtM5NyCnC0rLLayTcen4Xx1P/nMygR9sVWYtJFxfuk9wo3Wfkn6xUu3o4ZJmZD+TvRtC6D nALURfNW/S4+ywB/CjPVKosi+ckgJZtPvYpNOLYbISrCv9FhDCwcaFYpfdiLZnkP4ddnWBOK9 RLlYMsSLcsFQeNwWzxS+F0urg3+/mMbQBB4+dhQBB0m8MErm/0OB0HhKPwdyxiRb/8gqsRdpa /HVxdg21pQCMGeb1DVgWqYCSlEkexruDQ5w2qeTAjD2XkeCgtgrOyyoF9/sRbxVHXTXBpq8fG 9SlULaaAepGKfm7nLrqAfVWCiETzF09qFi9uzAhaqqbHj5ygtYkP0c/aB7V6uhoQovPDnheBK nLMD+NkEzUS9GWMnTgy2YsUS68uNRN7OgT9DyC/R48pAFnMDWxut0CYcWs7htuYhJ2EdqtNkw fKCBvYpBYIj1R5A6x1UvCmtpq4zA94v3i2J7HmfiI5DEdI1iIsYbOCXud4Rn/sUMfpK9veIsh tSnqu0s3H2ySHoXnN/cNF+wRjDmJohiZko/tqS+Y8xfZ4IaLhaRs+49WZ19haHOlTs8yPtdor 3UOwofokYidH9S2zPpz35UVq9Gb9MAbImknbdmIwlE23ZLYuv1rstWAeVKw1gARefXra+sy/R 3qS0o1R6kbSHwHnjd4+L0u/C/bWgF0c6dcMH25QHFXEMhB0u5jZYtxsC6UuCdalyLRdklXJ
Archived-At: <https://mailarchive.ietf.org/arch/msg/netmod/rJjFS3DGMxXk64AoVS2tjWQFeMQ>
Subject: Re: [netmod] [yang-doctors] Yangdoctors last call review of draft-ietf-netmod-nmda-diff-04
X-BeenThere: netmod@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: NETMOD WG list <netmod.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/netmod>, <mailto:netmod-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/netmod/>
List-Post: <mailto:netmod@ietf.org>
List-Help: <mailto:netmod-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/netmod>, <mailto:netmod-request@ietf.org?subject=subscribe>
X-List-Received-Date: Fri, 18 Sep 2020 19:42:32 -0000

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 <ludwig@clemm.org>
> *Date: *Tuesday, September 15, 2020 at 7:31 PM
> *To: *"Reshad Rahman (rrahman)" <rrahman@cisco.com>om>,
> "yang-doctors@ietf.org" <yang-doctors@ietf.org>
> *Cc: *"last-call@ietf.org" <last-call@ietf.org>rg>, "netmod@ietf.org"
> <netmod@ietf.org>rg>, "draft-ietf-netmod-nmda-diff.all@ietf.org"
> <draft-ietf-netmod-nmda-diff.all@ietf.org>
> *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" <yang-doctors-bounces@ietf.org on behalf of ludwig@clemm.org> <mailto:yang-doctors-bounces@ietf.orgonbehalfofludwig@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.
>
>      
>
>      
>