Re: [Netconf] RGW YANG push -18 LC comments [part 2]

["Eric Voit (evoit)" <[e-mail]>]

Hi Robert,

Thanks for the comments.  In-line with <Eric>....

From: Robert Wilton, September 10, 2018 6:19 AM

Hi Alex, authors,

Here are my remaining comments from my WG LC review of the YANG push draft.  Again, I think that these are all minor.

1. Sec 4.3.2, paragraph 2.  "resulted in a particular push" => "resulted in a particular update record"?

<Eric> Agree, we should make this tweak.


2. Sec 4.4.1, after Figure 7, "MAY respond" => "may respond".  This is just an example from that normative behaviour.

<Eric> Agree, we should make this tweak.


3. Sec 4.4.1, paragraph starting "If a request is rejected ...".  Is it really right for this to be "SHOULD" rather than "MAY"?  Early text (e.g. 3.2. paragraph 2 uses "may be inserted".  Also in section 4.4.2, paragraph after Figure 11.

<Eric> I prefer SHOULD.  Section 3.2 text had “may” instead of “MAY”.


4. Sec 4.4.1, "Optionally, an "error-severity" of "error" (this MAY but does not have to be included).", I don't think that the last clause is required (given that it is predicated by Optionally, so suggest deleting the clause in parenthesis.  Otherwise it is also inconsistent with the following "error-info" description.  If this change, it should also be changed for "error-severity" in Sec 4.4.2.

<Eric> Agree, we should make this tweak.


5. Sec 4.4.1, NETCONF specific text about errors.  Should that text really be in this draft?  Would it not be nicer to keep all of the NETCONF specifics to a NETCONF protocol draft?  Perhaps this is not practical, but the WG is currently experiencing some pain where NETCONF protocol specific normative text is in RFC 7950.  Hence, it would be nice to avoid introducing that issue here if possible.  This same comment also applies to Sec 4.4.2.

<Eric> As this is a non-normative example, we should not hit the same issue.  I do like examples for readers to digest.


6. Sec 4.4.2, description paragraph.  Suggest the following change to match the actual example: "modify the "period" of a subscription." => "modify the "period" and "datastore XPath filter" of a subscription."

<Eric> Agree, we should make this tweak.


7. Sec 4.4.2, first paragraph after figure 11:  Possible rewording:

Old:


   The publisher MUST respond explicitly positively or negatively to the

   request.  If the subscription modification is rejected, the

   subscription is maintained as it was before the modification request.

   In addition, the publisher MUST send an RPC error response.  This RPC

   error response SHOULD contain hints encapsulated within the yang-data

   structure "modify-subscription-error-datastore".  A subscription MAY

   be modified multiple times.

NEW:


   The publisher MUST respond to the subscription modification request.

   If the request is rejected, the existing subscription is left unchanged, and

   the publisher MUST send an RPC error response, which MAY|SHOULD contain

   hints encapsulated within the yang-data structure

   "modify-subscription-error-datastore".  A subscription MAY be modified

   multiple times.

<Eric> Proposed text looks fine, with a small tweak:

The publisher MUST respond to the subscription modification request.  If the request is rejected, the existing subscription is left unchanged, and the publisher MUST send an RPC error response.  This response might have hints encapsulated within the yang-data structure "modify-subscription-error-datastore".  A subscription MAY be modified multiple times.


8. Sec 4.4.4, no figure number under the example.

<Eric> Agree.  Alex, can you tweak the XML?


9. Sec 4.4.5, paragraph 1:
I think that it would be better if this just referred to YANG library, hence suggest changing the first two paragraphs:

OLD:


   To make subscription requests, the subscriber needs to know the YANG

   module library available on the publisher.  The YANG 1 module library

   information is sent by a NETCONF server in the NETCONF "hello"

   message.  For YANG 1.1 modules and all modules used with the RESTCONF

   [RFC8040] protocol, this information is provided by the YANG Library

   module (ietf-yang-library.yang from [RFC7895].  This YANG library

   information is important for the receiver to reproduce the set of

   object definitions used within the publisher.



   The YANG library includes a module list with the name, revision,

   enabled features, and applied deviations for each YANG module

   implemented by the publisher.  The receiver is expected to know the

   YANG library information before starting a subscription.

NEW:


   To make subscription requests, the subscriber needs to know the YANG

   datastore schemas used by the publisher, which are available via the YANG

   Library module, ietf-yang-library.yang from [RFC7895].  The receiver

   is expected to know the YANG library information before starting a

   subscription.

<Eric> Agree, we should make this tweak.


YANG module:

10. Top level description statement, it looks like the copyright has accidentally been indented by 4 chars.

<Eric> Agree, we should make this tweak.


Identities:
11. I suggest renaming "no-such-subscription-resynch" to just "no-such-subscription".  I would have thought that this error could also be returned to cancel an unknown subscription.

<eric> The reason we need "no-such-subscription-resynch" is that "no-such-subscription" is already used in SN.  (I.e., is not possible to augment the existing identity with a new base.)


12. "synchronization-size" doesn't sound like an error, perhaps "synchronization-too-big" or "resync-too-big" might be better?

<eric> I have no problem with changing it to "synchronization-too-big", that aligns to the other identity "update-too-big"


Type definitions:

13.  Change all instances (create, delete, insert, move, replace)  of "data node" to "datastore node".

14. grouping selection-filter-types, anydata datastore-subtree-filter, perhaps add a reference to "RFC 6241: Network Configuration Protocol, Section 6.", mirroring draft-ietf-netconf-nmda-netconf.

15. grouping update-policy-modifiable, container on-change description statement, needs to be indented by 2 spaces.

16. grouping hints, description, indent the second description line by 1 space.

17. rpc resynch-subscription, description statement, last two lines require alignment.


<eric> I think all five suggestions above are good tweaks.

Eric


Thanks,
Rob