Re: [Netconf] Shepherd review of draft-ietf-netconf-nmda-restconf

[Martin Bjorklund <[e-mail]>]

Mahesh Jethanandani <[e-mail]> wrote:
> 
> 
> > On Apr 12, 2018, at 3:53 AM, Martin Bjorklund <[e-mail]> wrote:
> > 
> > Hi,
> > 
> > 
> > Mahesh Jethanandani <[e-mail]> wrote:
> >> I have reviewed -03 version of draft-ietf-netconf-nmda-restconf. 
> >> 
> >> The document is well written, and is easy to read, but has issues
> >> which should be addressed before the document is sent to IESG.
> >> 
> >> Nits.
> >> 
> >> Abstract:
> >> 
> >> s/REF Editor/RFC Editor/
> > 
> > Fixed.
> > 
> >> 1.0 Introduction:
> >> 
> >> The first comma in this sentence seems to be misplaced. 
> >> 
> >> “supported in each datastore and, finally,” ??
> > 
> > I removed the "finally,".
> > 
> >> 2. Datastore and YANG Library Requirements
> >> 
> >> Can we consolidate the RFC Editor instructions in one place, instead
> >> of splitting them between here and Abstract.
> > 
> > I don't think this matters; these are just instructions to the RFC
> > editor that will be removed.  If the RFC editor has a preference for
> > either style I'd be happy to follow that.
> 
> The RFC editors have indicated that they would prefer RFC instructions
> for the entire draft appear in one place.
> 
> > 
> >> Detailed comments:
> >> 
> >> Introduction:
> >> 
> >> The document states:
> >> 
> >> "This document updates [RFC8040 <https://tools.ietf.org/html/rfc8040>]
> >> in order to enable RESTCONF clients to discover which datastores are
> >> supported by the RESTCONF server, as well as determine which modules
> >> are supported in each datastore and, finally, to interact with all the
> >> datastores supported by the NMDA. Specifically, the update introduces
> >> new datastore resources, adds a new query parameter, and requires the
> >> usage of [I-D.ietf-netconf-rfc7895bis
> >> <https://tools.ietf.org/html/draft-ietf-netconf-nmda-restconf-03.html#ref-I-D.ietf-netconf-rfc7895bis>]
> >> by RESTCONF servers implementing the NMDA."
> >> 
> >> 
> >> Questions:
> >> 
> >> 1) How does this draft enable RESTCONF clients to discover which
> >> datastores are supported?
> >> 
> >> Is it the definition of the new datastores resource? If so, can that
> >> be made explicit, by saying something like this in Section 3.1
> > 
> > No, it is the inclusion of the new YANG library that makes this
> > possible.  I suggest we add a paragraph at the end of section 2:
> > 
> >  A RESTCONF client can discover which datastores and YANG modules the
> >  server supports by reading the YANG library information from the
> >  operational state datastore.
> 
> Ok.
> 
> > 
> > 
> >> OLD:
> >> This document defines a set of new resources representing datastores
> >> as defined in [I-D.ietf-netmod-revised-datastores
> >> <https://tools.ietf.org/html/draft-ietf-netconf-nmda-restconf-03.html#ref-I-D.ietf-netmod-revised-datastores>].
> >> 
> >> NEW:
> >> This document defines a set of new resources representing datastores
> >> as defined in [I-D.ietf-netmod-revised-datastores
> >> <https://tools.ietf.org/html/draft-ietf-netconf-nmda-restconf-03.html#ref-I-D.ietf-netmod-revised-datastores>]. This
> >> resource
> >> can be used to discover the datastores that are supported by the
> >> server.
> >> 
> >> On the question of discovery of datastores itself, would the clients
> >> use the HEAD or GET methods to query what datastores are supported? If
> >> so, an example would be nice. Specifically, something that explains
> >> what response and errors to expect if the datastore is supported, and
> >> what response and errors to expect if the queried datastore is not
> >> supported.
> >> 
> >> 2) How does the draft help determine which modules are supported by
> >> each datastore?
> > 
> > See above, this is also covered by my proposed text.
> > 
> >> I went looking for text in the draft that talks about module discovery
> >> under each datastore, but did not find any. Am I missing something?
> 
> I do not see this issue addressed. The document states in the
> Introduction that:
> 
> "as well as determine which modules are supported in each datastore"
> 
> Where in the draft are we talking about module discovery in each
> datastore? If this is covered somewhere else, we should either remove
> this sentence or refer to the draft that covers it.

The text I proposed was:

  A RESTCONF client can discover which datastores and YANG modules the
  server supports by reading the YANG library information from the
  operational state datastore.

Do you think this answers the question which modules are supported?


/martin


> 
> Thanks.
> 
> >> 
> >> 3) The part of the sentence “to interact with all the datastores
> >> supported by the NMDA” seems incomplete.
> > 
> >> What interaction are we referring to? Is it the protocol operations on
> >> the datastore? Is it the query parameters on the datastores?
> > 
> > Yes, both.  A client can interact by reading or writing the
> > datastores.
> > 
> >> Section 3.2.1
> >> 
> >> What happens if the capability is not announced, but a “with-defaults”
> >> query is issued?
> > 
> > If the server doesn't understand the query, it will reply as defined
> > by RFC 8040, section 4.8.  This document doesn't change that behavior.
> > 
> >> Finally, what are the implications of explicit discovery of datastores
> >> supported by a RESTCONF server? For example, if the above
> >> “with-defaults” query is issued with the “content” query parameter set
> >> to “nonconfig", default values from which datastore are returned?
> > 
> > I don't understand this question.  Neither of these query parameter
> > controls from which datastore data is returned.
> > 
> >> Also, if the query parameter “content” is not present, RFC 8040 says
> >> that the default value is “all”. Does “all” include operational
> >> datastore data nodes, with or without the new capability? In other
> >> words, do we need to redefine “all”?
> > 
> > No, RFC 8040 says that "all" means: "Return all descendant data
> > nodes".  This document doesn't change that.
> > 
> > 
> > /martin
> 
> Mahesh Jethanandani
> [e-mail]
>