[netmod] AD review of draft-ietf-netconf-notification-capabilities

2021-06-21 Thread Rob Wilton (rwilton)
Hi,

Here is my AD review of draft-ietf-netconf-notification-capabiltiies-16

Thanks for this draft, sorry for the delay in reviewing.  It looks like it is 
in good shape.

I think that most of my comments are minor or cosmetic suggestions to 
potentially improve the phrasing of the text.


1.
Abstract:

   The module "ietf-system-capabilities" provides a placeholder
   structure that can be used to discover YANG related system
   capabilities for servers.  The module can be used to report
   capability information from the server at run-time or implementation-
   time, per the YANG Instance Data File Format.

Suggest "by making use of" rather than "per".


2.
   1.  Introduction

   There is a need to publish this capability information as it is part
   of the contract between the server and client.

Suggest "contract" -> "API contract".


3.
   There is a need to publish this capability information as it is part
   of the contract between the server and client.  Examples include
   maximum size of data that can be stored or transferred, information
   about counters (whether a node supports "on-change" telemetry), etc.
   Such capabilities are often dependent on a vendor's implementation or
   the available resources at deployment.  Many such capabilities are
   specific to either the complete system, individual YANG datastores
   [RFC8342] or specific parts of the YANG schema, or even individual
   data nodes.  It is a goal of this document to provide a common way of
   representing such capabilities in a format that is:

Suggest: maximum -> the maximum
 "or specific" -> ", specific"


4.
   o  available in identical format both at implementation-time and run-
  time
  
Suggest: "in an identical", and a period at the end.


5.
   If the information is
   not documented in a way available to the NMS designer, but only as
   instance data from the network node once it is deployed, the NMS
   implementation will be delayed

Suggest: "way available" => "way that is readily available"


6.
   The network operator needs to plan his
   management practices and NMS implementation before he even decides to
   buy the specific network node type.

Suggest: "him" -> "their", "he even decides" -> "they decide"


7.
   Run-time information is needed:
   
Suggest: Run-time capability information is needed:


8.
   o  to check that capability information provided earlier, at
  implementation-time is what the publisher has implemented.

Suggest: "at implementation-time, is"


9.
 To find a capability value for a specific data node in a
 specific datastore the user SHALL:
 
Please clarify that the capability value is selected by the relative path
to the datanode defining the capability.  i.e., the same name/path must be
used both under the system level and per datastore level capabilties.


10.
 2) If the datastore entry is found within that entry, process all
 per-node-capabilities entries in the order they appear in the list.
 The first entry that specifies the specific capability and has a
 node-selector selecting the specific data node defines the
 capability value.

I'm not sure this is required, but perhaps consider adding text to make it clear
that longest path matching can be achieved by ordering more specific
matches before less specific matches.


11.
// augmentation point for system level capabilities
Suggest: "Augmentation ... capabilities."  I would also suggest using a block 
style
comment so this doesn't get lost.   


12. 
   Only one specific datastore can be specified
   e.g., ds:conventional is not allowed.";
   
Suggest changing to:

   Only specific datastores can be specified.
   E.g., ds:conventional, which represents a
   set of configuration datastores, must not be
   used";


13.
  description
"A method to select all or some nodes within a datastore.";

"some or all" would flow better.


14.
// augmentation point for datastore or data node level
// capabilities

Suggest: "Augmentation ... capabilities."  I would also suggest using a block 
style
comment so this doesn't get lost.


15.
5.2.  YANG Module (ietf-notification-capabilities)

  - capabilities related to the throughput of notification data
  the publisher can support. (Note that for a specific
  subscription the publisher MAY still allow only longer periods
  or smaller updates depending on e.g., actual load conditions.)
  
Suggest: "data that the publisher"
 "specific subscription, the"
 "still allow" -> "allow"
 "e.g., -> ", e.g., "


16.
   bit config-changes {
 description
   "The publisher is capable of sending
notifications for 'config false' nodes for the
relevant scope and subscription type.";
 

Re: [netmod] AD review of draft-ietf-netconf-notification-capabilities

2021-07-05 Thread Benoit Claise

Hi Rob,

Thanks for your detailed review.
A new draft version has been posted.

URL:https://www.ietf.org/archive/id/draft-ietf-netconf-notification-capabilities-17.txt
Status:https://datatracker.ietf.org/doc/draft-ietf-netconf-notification-capabilities/
Htmlized:https://datatracker.ietf.org/doc/html/draft-ietf-netconf-notification-capabilities
Diff:https://www.ietf.org/rfcdiff?url2=draft-ietf-netconf-notification-capabilities-17


See the justifications below.

On 6/21/2021 10:45 AM, Rob Wilton (rwilton) wrote:

Hi,

Here is my AD review of draft-ietf-netconf-notification-capabiltiies-16

Thanks for this draft, sorry for the delay in reviewing.  It looks like it is 
in good shape.

I think that most of my comments are minor or cosmetic suggestions to 
potentially improve the phrasing of the text.


1.
Abstract:

The module "ietf-system-capabilities" provides a placeholder
structure that can be used to discover YANG related system
capabilities for servers.  The module can be used to report
capability information from the server at run-time or implementation-
time, per the YANG Instance Data File Format.

Suggest "by making use of" rather than "per".

DONE.



2.
1.  Introduction

There is a need to publish this capability information as it is part
of the contract between the server and client.

Suggest "contract" -> "API contract".

DONE



3.
There is a need to publish this capability information as it is part
of the contract between the server and client.  Examples include
maximum size of data that can be stored or transferred, information
about counters (whether a node supports "on-change" telemetry), etc.
Such capabilities are often dependent on a vendor's implementation or
the available resources at deployment.  Many such capabilities are
specific to either the complete system, individual YANG datastores
[RFC8342] or specific parts of the YANG schema, or even individual
data nodes.  It is a goal of this document to provide a common way of
representing such capabilities in a format that is:

Suggest: maximum -> the maximum
  "or specific" -> ", specific"

DONE



4.
o  available in identical format both at implementation-time and run-
   time

Suggest: "in an identical", and a period at the end.

DONE



5.
If the information is
not documented in a way available to the NMS designer, but only as
instance data from the network node once it is deployed, the NMS
implementation will be delayed

Suggest: "way available" => "way that is readily available"

DONE



6.
The network operator needs to plan his
management practices and NMS implementation before he even decides to
buy the specific network node type.

Suggest: "him" -> "their", "he even decides" -> "they decide"

DONE



7.
Run-time information is needed:

Suggest: Run-time capability information is needed:

DONE.



8.
o  to check that capability information provided earlier, at
   implementation-time is what the publisher has implemented.

Suggest: "at implementation-time, is"

DONE



9.
  To find a capability value for a specific data node in a
  specific datastore the user SHALL:

Please clarify that the capability value is selected by the relative path
to the datanode defining the capability.  i.e., the same name/path must be
used both under the system level and per datastore level capabilties.

NEW SENTENCE.

"When stating a specific capability, the relative path for any specific
capability must be the same under the system-capabilities container and
under the per-node-capabilities list: the same grouping for defining the
capabilities MUST be used. "


10.
2) If the datastore entry is found within that entry, process all
  per-node-capabilities entries in the order they appear in the list.
  The first entry that specifies the specific capability and has a
  node-selector selecting the specific data node defines the
  capability value.

I'm not sure this is required, but perhaps consider adding text to make it clear
that longest path matching can be achieved by ordering more specific
matches before less specific matches.
ADDED "Note that longest path matching can be achieved by ordering more 
specific matches before less specific ones"

under

list per-node-capabilities {
description
  "Each list entry specifies capabilities for the selected
   data nodes. The same capabilities apply for the data nodes
   in the subtree below the selected nodes.

   The system SHALL order the entries according to their
   precedence. The order of the entries MUST NOT change unless
   the underlying capabilities also change.";

We did NOT add next to "2) If the datastore entry is found within that 
entry ..." because that section focuses on the user (as opposed to the 
implementer on the server), as mentioned in "To find a capability 

Re: [netmod] AD review of draft-ietf-netconf-notification-capabilities

2021-07-05 Thread Rob Wilton (rwilton)
Hi Benoit,

Thanks for accommodating my suggestions.  All resolutions look good to me.

I have a preference to hold shipping this document to IETF LC until the 
instance-data doc is also ready.  I think that it will help the IESG review if 
they have both documents available to review at the same time.  I assume that 
the updates to the instance-data doc are also in progress?

Thanks,
Rob


From: Benoit Claise 
Sent: 05 July 2021 11:53
To: Rob Wilton (rwilton) ; netmod@ietf.org; 
draft-ietf-netconf-notification-capabilit...@ietf.org
Cc: NetMod WG Chairs 
Subject: Re: AD review of draft-ietf-netconf-notification-capabilities

Hi Rob,

Thanks for your detailed review.
A new draft version has been posted.


URL:
https://www.ietf.org/archive/id/draft-ietf-netconf-notification-capabilities-17.txt

Status: 
https://datatracker.ietf.org/doc/draft-ietf-netconf-notification-capabilities/

Htmlized:   
https://datatracker.ietf.org/doc/html/draft-ietf-netconf-notification-capabilities

Diff:   
https://www.ietf.org/rfcdiff?url2=draft-ietf-netconf-notification-capabilities-17

See the justifications below.
On 6/21/2021 10:45 AM, Rob Wilton (rwilton) wrote:

Hi,



Here is my AD review of draft-ietf-netconf-notification-capabiltiies-16



Thanks for this draft, sorry for the delay in reviewing.  It looks like it is 
in good shape.



I think that most of my comments are minor or cosmetic suggestions to 
potentially improve the phrasing of the text.





1.

Abstract:



   The module "ietf-system-capabilities" provides a placeholder

   structure that can be used to discover YANG related system

   capabilities for servers.  The module can be used to report

   capability information from the server at run-time or implementation-

   time, per the YANG Instance Data File Format.



Suggest "by making use of" rather than "per".
DONE.








2.

   1.  Introduction



   There is a need to publish this capability information as it is part

   of the contract between the server and client.



Suggest "contract" -> "API contract".
DONE








3.

   There is a need to publish this capability information as it is part

   of the contract between the server and client.  Examples include

   maximum size of data that can be stored or transferred, information

   about counters (whether a node supports "on-change" telemetry), etc.

   Such capabilities are often dependent on a vendor's implementation or

   the available resources at deployment.  Many such capabilities are

   specific to either the complete system, individual YANG datastores

   [RFC8342] or specific parts of the YANG schema, or even individual

   data nodes.  It is a goal of this document to provide a common way of

   representing such capabilities in a format that is:



Suggest: maximum -> the maximum

 "or specific" -> ", specific"
DONE








4.

   o  available in identical format both at implementation-time and run-

  time



Suggest: "in an identical", and a period at the end.
DONE








5.

   If the information is

   not documented in a way available to the NMS designer, but only as

   instance data from the network node once it is deployed, the NMS

   implementation will be delayed



Suggest: "way available" => "way that is readily available"
DONE








6.

   The network operator needs to plan his

   management practices and NMS implementation before he even decides to

   buy the specific network node type.



Suggest: "him" -> "their", "he even decides" -> "they decide"
DONE








7.

   Run-time information is needed:



Suggest: Run-time capability information is needed:
DONE.








8.

   o  to check that capability information provided earlier, at

  implementation-time is what the publisher has implemented.



Suggest: "at implementation-time, is"
DONE








9.

 To find a capability value for a specific data node in a

 specific datastore the user SHALL:



Please clarify that the capability value is selected by the relative path

to the datanode defining the capability.  i.e., the same name/path must be

used both under the system level and per datastore level capabilties.
NEW SENTENCE.


"When stating a specific capability, the relative path for any specific

capability must be the same under the system-capabilities container and

under the per-node-capabilities list: the same grouping for defining the

capabilities MUST be used. "

10.

2) If the datastore entry is found within that entry, process all

 per-node-capabilities entries in the order they appear in the list.

 The first entry that specifies the specific capability and has a

 node-selector selecting the specific data node defines the

 capability value.



I'm not sure this is required, but perhaps consider adding text to make it clear

that longest path matching can be achieved by ordering more specific

matches before less specific matches.
ADDED "Note that longest path matching ca