On Tue, May 16, 2017 at 1:53 PM, Eric Voit (evoit) <evoit@xxxxxxxxx> wrote:
Hi Bert,
Thanks very much for the review. Some thoughts in-line...
> From: Bert Wijnen, May 16, 2017 8:38 AM
>
> Reviewer: Bert Wijnen
> Review result: On the Right Track
>
>
> - last para of sect 3.5.
> This seems to me to make it difficult to create interoperable
> implementations. Or is there a way for a client to figure out what
> is or is not support, other than tryal and error?
Minimal XPATH syntax support is really a subscription independent issue. At this time, I am not aware of anyone attempting to constrain which XPATH capabilities should be the minimum set for the industry. One of the reasons is that going with a minimum common
denominator for a high volume information set (like routing changes) might constrain the availability of low volume, high value filters (like configuration changes). I would very much welcome someone who wanted to attempt work in this area. As David
Waltermire said to me on this topic today, it might be possible to identify a minimum XPATH filter capability set for various roles in the network element. But this is non-trivial work.
For now, I am assuming any vendor will be able to articulate what the syntax capabilities are for their platforms. And they can do outside the standards arena. A good way to do this would be to pre-populate example filters in the "filters" objects.
This is not going to be interoperable or easy to debug to find out what a vendor supports.
Sec 3.5, para 3 & 4 about XPath implementations should be replaced. The text should say a node-set result
MUST be generated or the result is the empty node-set.
Some XPath constructs SHOULD be avoided, as specified in sec 4.5 of RFC 6087.
<evoit> makes sense. The reference to RFC 6087 is a good one
Filters need to be well-defined so clients can actually use them.
The same syntax as a NETCONF <get> XPath filter should be used here.
<evoit> Yes. We always are aiming with equivalence to <get>
New filters should be defined if they are really needed.
<evoit> Over time, new filter types can be augmented in as identities. Again, <get> equivalence is the goal. Certainly adding them into the model here is the
easy part vs dimensioning what a platform can support.
Eric
> - page 41:
>
> /* YANG Parser Pyang crashing on the following syntax below
>
> So does the definition get skipped? Or what needs to happen here?
There is a coming in pyang 1.7.2 which fixes the bug...
https://github.com/mbj4668/pyang/issues/300
See
https://github.com/mbj4668/pyang/commit/b891cc3dd3a4547f9eddd83882e9872bc2066c6d
All we need to do is await the new version.
> Consistency
>
> - last bullet on page 7 talks about "YANG subtrees". I do not see that term
> in netconf or yang documents. Those just talk about "subtrees".
> Maybe I am
> not looking good enough?
Will remove the word YANG
> - top of page 8 I see the words "xpath", "Xpath" and "XPath"
> is there a difference?
No. Will fix.
> Nits
Nice catches. Will fix the items below.
Eric
> - you may want to check the reference/citation occurrences of [subscribe]
> at several places it points to
> draft-ietf-netconf-yang-push-06#ref-subscribe
> whereas I think it intends to point to the [subscribe] in the
> normative references section
> - first bullet on page 5:
> Enhancements to filters. Specifically the filter MUST at identify at
> least one targeted yang
> s/at// -- the first "at" seems superfluous
> plus, you are using capitalized MUST with out reference/citation of
> RFC2119
> - page 36:
>
> leaf dependency {
> type sn:subscription-id;
> description
> "Provides the Subscription ID of a parent subscription which
> has absolute priority should that parent have push updates
> ready to egress the publisher. In other words, there should be
> no streaming of objects from the current subscription if of
> the parent has something ready to push.";
> reference
> "RFC-7540, section 5.3.1";
> }
>
> s/if of/if/ ??