Re: [Netconf] I-D Action: draft-ietf-netconf-yang-patch-11.txt

[Benoit Claise <[e-mail]>]

Andy,
>
>
> On Wed, Sep 21, 2016 at 9:53 AM, Benoit Claise <[e-mail] 
> <mailto:[e-mail]>> wrote:
>
>     Dear authors,
>
>     Even if I saw "all these issues addressed in github version" (at
>     https://github.com/netconf-wg/yang-patch/issues/10
>     <https://github.com/netconf-wg/yang-patch/issues/10>), I've been
>     checking whether all my "AD review:
>     draft-ietf-netconf-yang-patch-10" feedback has been taken into
>     account.
>
>     - I see the new definition:
>
>     YANG Patch template: this is similar to a YANG data template,
>            except it has a representation with the media type "application/
>            yang-patch-xml" or "application/yang-patch+json".
>
>     YANG data template is defined in [I-D.ietf-netconf-restconf
>     <https://tools.ietf.org/html/draft-ietf-netconf-yang-patch-11#ref-I-D.ietf-netconf-restconf>]
>     according to section 1.1.4
>     However, I don't find this term in the RESTCONF draft.
>
>
> YANG data template is defined in RESTCONF.
> YANG Patch template is defined in YANG Patch.
> RESTCONF never uses the term YANG Patch template.
> Why is this a problem?
My bad. Sorry
I blindly followed the link from the YANG Patch normative references, 
and looked up the RESTCONF draft v13.

>
>
>     - I'm not sure how these two pieces of feedback have been resolved:
>
>         Btw, I guess that nothing prevents a RESTCONF server to
>         support simultaneously the plain PATCH (
>         https://tools.ietf.org/html/draft-ietf-netconf-restconf-14#section-4.6
>         <https://tools.ietf.org/html/draft-ietf-netconf-restconf-14#section-4.6>)
>         and the PATCH in this document? However, doing so, would
>         provide a useless audit log, since the plain PATCH doesn't
>         have a patch-id
>
>                leaf patch-id {
>                     type string;
>                     description
>                       "An arbitrary string provided by the client to identify
>                        the entire patch.  This value SHOULD be present in any
>                        audit logging records generated by the server for the
>                        patch. Error messages returned by the server pertaining
>                        to this patch will be identified by this patch-id value.";
>                   }
>
>         Worth writing some text about it?
>
>
>
> There is no standard audit log so to say the audit log cannot 
> differentiate
> between a plain and YANG PATCH is pure speculation about an
> implementation.
Ok.
>
>
>         leaf comment {
>                     type string;
>                     description
>                       "An arbitrary string provided by the client to describe
>                        the entire patch.  This value SHOULD be present in any
>                        audit logging records generated by the server for the
>                        patch.";
>
>
>         I don't understand why it's not a MUST. Is this because this
>         field is optional?
>         So if it's not in the audit logging, we don't know if it's
>         because the field is empty, or because someone decided not to
>         include it.
>         Do you want to say: If populated, this value MUST be present
>         in any audit logging records generated by the server for the
>         patch.
>
>
>
>
> There is no standard audit log.
> A server implementation MAY toss the comments since they do not
> impact the operation.
>
> If the IETF wants to standardize audit logging then that
> should be properly, and not place MUST requirements
> to support undefined mechanisms.
>
> Why should this draft mandate the structure of proprietary
> audit logging solutions?  SHOULD seems more appropriate.
> If there was a standard logging mechanism that had a <comment> field,
> then it would be OK to say "implementations of the ietf-audit-log module
> MUST populate the <comment> field with the contents of this field".
>
>
Ok.
>
>     - And I see this diff between version 8 and 9.
>            
>
>
>     Surprised that, if there is any error, it's not a MUST to return the "yang-patch-status" message?
>     Maybe I missed the discussion on the mailing list. Feel free to let me know.
>
> In our implementation there are errors handled before the "YANG Patch 
> code" ever sees
> the internal representation of the message. e.g, "405 Method Not 
> Allowed". 
When I see "If a YANG Patch is completed with errors, the RESTCONF 
server SHOULD return a "yang-patch-status" message.", I'm concerned that 
no error message would be sent to the RESTCONF client if a YANG Patch is 
completed with errors. It's good practice, when you have a SHOULD to 
explain the exception(s) (btw, same remark for "If a YANG Patch is 
completed without errors, the RESTCONF server SHOULD return a 
"yang-patch-status" message"). I guess that you are after some text such 
as:

    "If a YANG Patch is completed with errors, the RESTCONF server MUST
    return a "yang-patch-status" message, unless a previous related
    error is already sent by the RESTCONF server, e.g, "405 Method Not
    Allowed". 

Regards, Benoit
>
>     Regards, Benoit
>
> Andy
>
>>     A New Internet-Draft is available from the on-line Internet-Drafts directories.
>>     This draft is a work item of the Network Configuration of the IETF.
>>
>>              Title           : YANG Patch Media Type
>>              Authors         : Andy Bierman
>>                                Martin Bjorklund
>>                                Kent Watsen
>>     	Filename        : draft-ietf-netconf-yang-patch-11.txt
>>     	Pages           : 41
>>     	Date            : 2016-08-15
>>
>>     Abstract:
>>         This document describes a method for applying patches to
>>         configuration datastores using data defined with the YANG data
>>         modeling language.
>>
>>
>>     The IETF datatracker status page for this draft is:
>>     https://datatracker.ietf.org/doc/draft-ietf-netconf-yang-patch/
>>     <https://datatracker.ietf.org/doc/draft-ietf-netconf-yang-patch/>
>>
>>     There's also a htmlized version available at:
>>     https://tools.ietf.org/html/draft-ietf-netconf-yang-patch-11
>>     <https://tools.ietf.org/html/draft-ietf-netconf-yang-patch-11>
>>
>>     A diff from the previous version is available at:
>>     https://www.ietf.org/rfcdiff?url2=draft-ietf-netconf-yang-patch-11
>>     <https://www.ietf.org/rfcdiff?url2=draft-ietf-netconf-yang-patch-11>
>>
>>
>>     Please note that it may take a couple of minutes from the time of submission
>>     until the htmlized version and diff are available attools.ietf.org <http://tools.ietf.org>.
>>
>>     Internet-Drafts are also available by anonymous FTP at:
>>     ftp://ftp.ietf.org/internet-drafts/
>>     <ftp://ftp.ietf.org/internet-drafts/>
>>
>>     _______________________________________________
>>     Netconf mailing list
>>     [e-mail] <mailto:[e-mail]>
>>     https://www.ietf.org/mailman/listinfo/netconf
>>     <https://www.ietf.org/mailman/listinfo/netconf>
>