Reviewer: Ebben Aries
Review result: Almost Ready
1 module in this draft:
- ietf-netconf-nmda@xxxxxxxxxxxxxxx
YANG validation errors or warnings (from pyang 1.7.3 and yanglint 0.14.69)
- ietf-netconf-nmda@xxxxxxxxxxxxxxx:171: warning: RFC 6087: 4.10,4.12:
statement "enum" should have a "description" substatement (From pyang 1.7.3)
0 examples are provided in this draft (section 3.12 of
draft-ietf-netmod-rfc6087bis-18)
Module ietf-netconf-nmda@xxxxxxxxxxxxxxx:
- Note: For the following imports, there are no references to the supporting
documents as suggested in rfc6087bis however this item is currently under
discussion on both usefulness/possible formatting
- import "ietf-yang-types" should reference RFC 6991 per (not as a comment)
https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-18#section-4.7
- import "ietf-inet-types" should reference RFC 6991 per (not as a comment)
https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-18#section-4.7
- import "ietf-datastores" should reference I-D.ietf-netmod-revised-datastores per
https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-18#section-4.7
- import "ietf-origin" should reference I-D.ietf-netmod-revised-datastores per
https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-18#section-4.7
- import "ietf-netconf" should reference RFC 6241 per
https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-18#section-4.7
- import "ietf-netconf-with-defaults" should reference RFC 6243 per
https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-18#section-4.7
- Module description, revision and feature definition should contain note to
RFC Ed. to change placeholder reference to RFC when assigned
https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-18#appendix-C
- Security Considerations looks good and is adjusted to account for NETCONF
only as well as addressing the additional RPCs introduced
- L82: suggested replacement of text:
s/, i.e., the filter criteria are logically ANDed/. Multiple filters are
processed as a logical AND operation/
- L127: suggested replacement of text:
s/then the get-data/the get-data/
General comments/nits on draft text:
- As mentioned above, there are 0 examples in this draft. There should be XML
RPC examples of the 2 newly defined RPCs as well as usage examples of the
augments of RFC6241 RPCs
- :with-origin urn:ietf:params:netconf:capability:with-origin:1.0
Wouldn't it make more sense to have this URN defined in
I-D.ietf-netmod-revised-datastores rather? In addition, should this
capability and feature be rather defined as 'origin' in the 'ietf-origin'
module? As defined today, this feature is used as a clause for
'origin-filter' as well as 'with-origin' inputs to get-data however these
inputs appear to be mutually exclusive.
IMHO, this should be detached from this specific draft which focuses on
NETCONF specific base extensions for datastores as the returning and
filtering of origin metadata can be transport independent.
- L308: suggested replacement of text:
s/parameter is optional to support/parameter is optional and dependant off
of the servers support for the 'origin' feature/ (dependant off the previous
comment)
- L320: "ietf-netconf-nmda" (see Section 4). - This should be an actual
reference as in line #306
- L345: "default-operation" parameter of the <edit-config> operation - This
should at a bare minimum call reference to the appropriate section in
RFC6241
- To the previous point, since there is a large amount of reference and
comparison to RFC6241 definitions, any references should be tagged
appropriately
