Subject: [Netconf] I-D Action: draft-ietf-netconf-restconf-11.txt

From: [email-masked]
Date: Sun, 10 Apr 2016 19:33:23 -0700

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           : RESTCONF Protocol
        Authors         : Andy Bierman
                          Martin Bjorklund
                          Kent Watsen
	Filename        : draft-ietf-netconf-restconf-11.txt
	Pages           : 116
	Date            : 2016-04-10

Abstract:
   This document describes an HTTP-based protocol that provides a
   programmatic interface for accessing data defined in YANG, using the
   datastores defined in NETCONF.

The IETF datatracker status page for this draft is:
https://datatracker.ietf.org/doc/draft-ietf-netconf-restconf/

There's also a htmlized version available at:
https://tools.ietf.org/html/draft-ietf-netconf-restconf-11

A diff from the previous version is available at:
https://www.ietf.org/rfcdiff?url2=draft-ietf-netconf-restconf-11

Please note that it may take a couple of minutes from the time of submission
until the htmlized version and diff are available at tools.ietf.org.

Internet-Drafts are also available by anonymous FTP at:
ftp://ftp.ietf.org/internet-drafts/

---

From: Andy Bierman <[email-masked]>
Date: Sun, 10 Apr 2016 19:36:59 -0700

Hi,

This draft-11 addresses comments on draft-10 from Lada and Tom Petch.
(issues 56 - 58 on https://github.com/netconf-wg/restconf/issues)

The RESTCONF, YANG Patch, and YANG Library drafts are now complete
and  ready for IESG and final IETF review.

Andy

On Sun, Apr 10, 2016 at 7:33 PM, <[email-masked]> wrote:

> _______________________________________________
> I-D-Announce mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/i-d-announce
> Internet-Draft directories: http://www.ietf.org/shadow.html
> or ftp://ftp.ietf.org/ietf/1shadow-sites.txt
>

---

From: Robert Wilton <[email-masked]>
Date: Mon, 11 Apr 2016 10:35:23 +0100

Hi Andy,

Did you have a chance to consider my comments or incorporate my feedback 
on RESTCONF 9/10 (attached)?

Thanks,
Rob

On 11/04/2016 03:36, Andy Bierman wrote:
> On Sun, Apr 10, 2016 at 7:33 PM, <[email-masked] 
> <mailto:[email-masked]>> wrote:
>
>
>     until the htmlized version and diff are available at
>     tools.ietf.org <http://tools.ietf.org>.
>
>     [email-masked] <mailto:[email-masked]>
>     Internet-Draft
>     <https://www.ietf.org/mailman/listinfo/i-d-announce%0AInternet-Draft>
>     directories: http://www.ietf.org/shadow.html
> Netconf mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/netconf

Subject: Re: [Netconf] RESTCONF-10 draft
From: Robert Wilton <[email-masked]>
To: Andy Bierman <[email-masked]>, Netconf <[email-masked]>
Date: Tue, 22 Mar 2016 18:10:50 +0000
List-Id: Network Configuration WG mailing list <netconf.ietf.org>

Hi Andy, Kent, Martin,

I think that you have addresses some of my recent comments on 09, but 
not all of them.

I've reproduced what I think are my outstanding review comments here 
which I think also apply to -10;

The first 6 questions related to section 4.8: Query Parameters:
1. Are query parameters and paths case sensitive?  I would guess so.  
The draft doesn't make any explicit mention of this, does it need to?

2. Should an unexpected query parameter cause an error or should it just 
be ignored?  For some (but not all) of the query parameters in sections 
4.8.1 through 4.8.9 it indicates that an explicit error should be 
returned if the query parameter is used for an resource type on which it 
isn't supported, but section 4.8 allows vendor specific query parameters 
to also be defined.  I think that interop might be easier if unknown 
query parameters are just ignored.

3. Both the depth and fields query parameters (4.8.2 & 4.8.3) indicate 
that they apply to the API resource (3.3), but I wasn't sure that they 
served any purpose since my reading of the draft is that the api 
resource is just the top level and effectively fixed. I.e. it doesn't 
return any data from the datastore or the operations, and they need to 
be explicitly queried separately if required.  Proposed fix:

4.8.2 & 4.8.3 Old:

    This parameter is only allowed for GET methods on*API,*  datastore, and
    data resources.  A "400 Bad Request" status-line is returned if it
    used for other methods or resource types.

4.8.2 & 4.8.3 New:

    This parameter is only allowed for GET methods on datastore, and
    data resources.  A "400 Bad Request" status-line is returned if it
    used for other methods or resource types.

4. Minor nit.  It might be helpful if the options were listed in the 
same order as they are listed in the table in section 4.8.  E.g. this 
would mean that the section on "filter" would need to move up above the 
section on "insert".

5. Section 4.8.2: The "depth" Query Parameter
The last but one paragraph reads:

    "By default, the server will include all sub-resources within a
    retrieved resource,*which have the same resource type as the requested resource. Only one 
level of sub-resources with a different media type than the target 
resource will be returned.*"

I'm not convinced that this paragraph is correct, e.g. for a GET request 
on /restconf/data.  By default I would expect it to return all levels of 
child nodes (which have a different resource type) rather than just the 
first layer of children.

If the depth parameter is changed to not apply to the API resource, then 
could this paragraph just be changed to:

New:

    "By default, the server will include all sub-resources within a
    retrieved resource."

6. Section 8: "error-info". Should the type be anydata rather than the 
deprecated anyxml?

Old:

            *anyxml *error-info {
               description
                 "This*anyxml *value MUST represent a container with
                 zero or more data nodes representing additional
                 error information.";
            }

New:

            anydata error-info {
               description
                 "This anydata value MUST represent a container with
                 zero or more data nodes representing additional
                 error information.";
            }

These remaining questions are those that I raised previously (eliding 
those which have been addressed in -10):

7. Section 3.4.1 Edit Collision Detection:
In -10, this has been updated to make it clear that it only applies to 
configuration resources which is now internally consistent.

However, it feels like the timestamp is potentially be a bit inaccurate 
for a REST like interface.

It is probably too late to raise this now, but I was wondering whether 
the server shouldn't also track operational resources using parallel 
timestamps/etags?  Whether the config or operational timestamp/etag is 
returned would depend on which node had been queried and the "content" 
GET query parameter:
  - If the content query was for "config" only then the config resources 
timestamp/etag would be returned.
  - if the content query was for "nonconfig" then the operational 
resources timestamp/etag would be returned.
  - If the content query was for "all" then the latest of the 
config/operational resource timestamp would be returned and a combined 
config/operational etag could be returned.  In addition, the config and 
operational timestamps and etags could be returned using separate HTTP 
headers.

8. Section 3.5 Data Resource, last paragraph:

    The resource definition version for a data resource is identified by
    the revision date of the YANG module containing the YANG definition
    for the data resource.

I don't understand the purpose of this last paragraph means, or what its 
relevance is.  I suggest that either "resource definition version" is 
defined in 1.1.4 Terms, or this paragraph is deleted.

9. Section 4.1. OPTIONS:

    The OPTIONS method is sent by the client to discover which methods
    are supported by the server for a specific resource (e.g., GET, POST,
    DELETE, etc.).

    The server SHOULD implement this method, however the same information
    could be extracted from the YANG modules and the RESTCONF protocol
    specification.

I suggest adding the sentence:

    The server SHOULD NOT include any body text for the OPTIONS method.

10. Section 7.1 Error Response message:

    When an error occurs for a request message on a*data*  resource or an
    *operation *resource, and a "4xx" class of status codes will be
    returned (except for status code "403 Forbidden"), then the server
    SHOULD send a response message-body containing the information
    described by the "errors" container definition within the YANG module
    Section 8 
<https://tools.ietf.org/html/draft-ietf-netconf-restconf-10#section-8>.  The Content-Type of this response message*MUST be application/yang.errors*  (see example below).

    The client*MAY *specify the desired encoding for error messages by
    specifying the appropriate media-type in the Accept header.*If no error media is specified, then the media type of the request 
message SHOULD be used, or the server MAY choose any supported message 
encoding format.*   If there is no request message the server MUST
    select "application/yang.errors+xml" or "application/
    yang.errors+json", depending on server preference.  All of the
    examples in this document, except for the one below, assume that XML
    encoding will be returned if there is an error.

i) The beginning of the error paragraph makes the description 
conditional on the request message being on a data resource.  Should 
this cover other resource types as well, or otherwise what is the error 
handling strategy for other resource types?
ii) Should the last sentence of the first paragraph be "The Content-Type 
of this response message MUST be a subtype of application/yang.errors 
(see example below). "?
iii) Would it be wise to indicate that the client SHOULD specify desired 
encoding for error messages?
iv) The second paragraph states "If no error media is specified, then 
the media type of the request message is used. " but this directly 
conflicts with "The Content- Type of this response message MUST be 
application/yang.errors (see example below). "
v) "If there is no request message the server ..." - I would think that 
there must always be a request message.

In summary, what about this text instead:

    When an error occurs for a request message on an*api, data,****datastore, or operation*  resource, and a "4xx" class of status codes will be
    returned (except for status code "403 Forbidden"), then the server
    SHOULD send a response message-body containing the information
    described by the "errors" container definition within the YANG module
    Section 8 
<https://tools.ietf.org/html/draft-ietf-netconf-restconf-10#section-8>.  The Content-Type of this response message*MUST**be a subtype of**application/yang.errors*  (see example below).

    The client*SHOULD *specify the desired encoding for error messages by
    specifying the appropriate media-type in the Accept header.*If no error media is specified, then the server SHOULD choose the error 
media****type with the clients preferred encoding, or otherwise the server MAY****choose any supported message encoding format. *If there is no request message
    the server MUST select "application/yang.errors+xml" or "application/
    yang.errors+json", depending on server preference.  All of the
    examples in this document, except for the one below, assume that XML
    encoding will be returned if there is an error.

Thanks,
Rob

On 20/03/2016 19:02, Andy Bierman wrote:
> Hi,
>
> Please review the latest version of the RESTCONF protocol:
> There have been many updates based on the extensive list
> of review comments:
>
> https://tools.ietf.org/html/draft-ietf-netconf-restconf-10#appendix-A.1
>
> This draft addresses all open issues found on github:
> https://github.com/netconf-wg/restconf/issues
>
> Not all requested edits were made. Please review the
> github issue tracker for details about the editing process.
>
>
> thanks,
> Andy
>
>
>
> Netconf mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/netconf

_______________________________________________
Netconf mailing list
[email-masked]
https://www.ietf.org/mailman/listinfo/netconf

---

From: Andy Bierman <[email-masked]>
Date: Mon, 11 Apr 2016 07:21:42 -0700

Hi,

Sorry I forgot these updates.
I will publish a draft-12 to make these clarifications.

Andy

On Mon, Apr 11, 2016 at 2:35 AM, Robert Wilton <[email-masked]> wrote:

> Netconf mailing [email-masked]://www.ietf.org/mailman/listinfo/netconf
>
>
>
>
> ---------- Forwarded message ----------
> Cc:
> I think that you have addresses some of my recent comments on 09, but not
> all of them.
>
> I've reproduced what I think are my outstanding review comments here which
> I think also apply to -10;
>
>
> 1. Are query parameters and paths case sensitive?  I would guess so.  The
> draft doesn't make any explicit mention of this, does it need to?
>
>
> 4.8.1 through 4.8.9 it indicates that an explicit error should be returned
> if the query parameter is used for an resource type on which it isn't
> supported, but section 4.8 allows vendor specific query parameters to also
> be defined.  I think that interop might be easier if unknown query
> parameters are just ignored.
>
>
> served any purpose since my reading of the draft is that the api resource
> is just the top level and effectively fixed.  I.e. it doesn't return any
> data from the datastore or the operations, and they need to be explicitly
> queried separately if required.  Proposed fix:
>
> 4.8.2 & 4.8.3 Old:
>
>    This parameter is only allowed for GET methods on *API,* datastore, and
> 4. Minor nit.  It might be helpful if the options were listed in the same
> order as they are listed in the table in section 4.8.  E.g. this would mean
> that the section on "filter" would need to move up above the section on
> "insert".
>
>
>    retrieved resource,* which have the same resource type as the
>    requested resource.  Only one level of sub-resources with a different
>    media type than the target resource will be returned.*"
>
>
>                 "This *anyxml *value MUST represent a container with
> It is probably too late to raise this now, but I was wondering whether the
> server shouldn't also track operational resources using parallel
> returned would depend on which node had been queried and the "content" GET
> query parameter:
>  - if the content query was for "nonconfig" then the operational resources
>    When an error occurs for a request message on a *data* resource or an
>    Section 8 <https://tools.ietf.org/html/draft-ietf-netconf-restconf-10#section-8>.  The Content-Type of this response message *MUST be
>    application/yang.errors* (see example below).
>
>    The client *MAY *specify the desired encoding for error messages by
>    specifying the appropriate media-type in the Accept header.  *If no
>    error media is specified, then the media type of the request message
>    SHOULD be used, or the server MAY choose any supported message
> i) The beginning of the error paragraph makes the description conditional
> on the request message being on a data resource.  Should this cover other
> resource types as well, or otherwise what is the error handling strategy
> for other resource types?
> of this response message MUST be a subtype of application/yang.errors (see
> example below). "?
> iv) The second paragraph states "If no error media is specified, then the
> media type of the request message is used. " but this directly conflicts
> with "The Content- Type of this response message MUST be
>    When an error occurs for a request message on an *api, data,**   datastore, or operation* resource, and a "4xx" class of status codes will be
>    Section 8 <https://tools.ietf.org/html/draft-ietf-netconf-restconf-10#section-8>.  The Content-Type of this response message *MUST** be a subtype of**
>    application/yang.errors* (see example below).
>
>    The client *SHOULD *specify the desired encoding for error messages by
>    specifying the appropriate media-type in the Accept header.  *If no
>    error media is specified, then the server SHOULD choose the error media**   type with the clients preferred encoding, or otherwise the server MAY**   choose any supported message encoding format. *If there is no request message
> Netconf mailing [email-masked]://www.ietf.org/mailman/listinfo/netconf
>
>
>

---

From: t.petch <[email-masked]>
Date: Tue, 12 Apr 2016 17:39:19 +0100

I like it!

That said, one substantive comment and some editorial quirks

s.11.4
The new registry needs a policy; suggest 'IETF Review'

s.6.3.1
"The YANG encoding MUST be converted to
   URL-encoded string for use in the request URI. "

The I-D uses a mix of URL and URI and I am unclear if there is intended
to be a difference - I would use URI throughout.

More basically, I struggle to see YANG as an encoding, rather a (data)
definition - most of the rest of the I-D has 'YANG defining' or 'YANG
definition' so I suggest

NEW "The YANG definition MUST be converted to a
   URI-encoded string for use in the request URI "

and with a reference back to 3.5.1 where all the details are specified.
I note that 3.5.1 has 'URL encoded' not 'URL-encoded'; I prefer the
hyphenated form and URI not URL.

In 3.6.1, in the same vein, I would also

OLD
In YANG, data nodes are named with an absolute XPath expression,

NEW
In YANG, data nodes are identified with an absolute XPath expression,

'named' does not seem right. 'identified' or 'defined'.

s.4.8.8
'   If the "replay-support" leaf is present in the "stream" entry
   (defined in Section 9.3) then the server MUST support the
   "start-time" and "stop-time" query parameters for that stream.'

this leaf is defined as a boolean so I think it better as

NEW
   If the "replay-support" leaf has the value 'true'  in the "stream"
entry

unless false booleans are elided, which is not what I understood to
happen.

s.7.1
"   body and hence Accept headers are typically not passed."

Is that meant to be 'parsed'?

Tom Petch