[netmod] ypath early review

[Kent Watsen <[e-mail]>]

Hi James,

Thank you for writing draft-jgc-netmod-yang-path.  
Some comments below.

Kent // contributor



Overarching:
I appreciate and agree with the general motivation for this work.  For instance, I too sometimes get caught by the instance-identifier syntax, and I generally want the YANG modeling language to drop all references to XPath.
The title is "YANG Path", which sounds right, but the work is definitely not limited to YANG, as a great deal of the text focuses on filters, which would only ever appear in protocols.
I question why so much text regards filters.  How pervasive are filters?  It seems that only NETCONF subtree filters are considered.  Maybe filters should be deemphasized in deference to the list pagination work?
YPath seems to focus on three things: schema node refs, data node refs, and filters.  I somewhat wish the document's structure was organized along these lines.  That said, some sections are already partitioned into schema/instance parts, which has a similar effect, but that structure not uniform throughout
Filters appear to be a variation of an instance data path.  In fact, the ABNF has "filter-predicate = instance-predicate".  This goes to my point above regarding structuring the document by schema node refs, data node refs, and filters...
What is the plan for supporting expressions?  It would seem odd for YANG2 to say when/must are XPaths , except the path-component is a YPath.  I think this document should have a section called "YPath Expressions"
I wish that there was an Appendix (to be removed by RFC Ed) called "Relation to Existing Work" where it enumerates how ypath applies to:
YANG (leafref, instance-identifier, etc.)
NETCONF (subtree filter, xpath filter, list pagination, etc.)
RESTCONF (URLs, list pagination, etc.)
YANG Push (selections, etc.)
Anything else?
Related, I wish the body of the document would say nothing about any of those things, except YANG of course.
In sum, a simpler path syntax in YANG2 would be great, for leafref, instance-identifier, must, when


Major:
Section 1:
The text says:
"However, some have shortcomings for YANG module authors, tool implementers, and operators who need a single, generic, human-readable path format that can<snip/>
I wish this section could be expanded to better explain the issues.  I'm familiar with module-author issues.  For tool-implementors, I wonder if this work would actually complicate things.  For operators, how is it an issue at all, if a GUI hides the ugly bits, or are you thinking specifically about operator scripts?
The text says:
"Selection based on the contents of node values (other than list keys) is out of scope."
My concern is that I think "must" and "when" expressions commonly refer to leafs other than list keys.
Oh, maybe here the text refers to inner-nodes in a ypath?  If so, then it could to stated more clearly.
Section 3.1:
The text says:
"A specification that splits a path into a prefix and a sub-path MUST evaluate both parts together from the root"
When in practice would a path be split into a prefix-path and a sub-path?
Section 3.5:
The text says:
"Imports and includes do not change the namespace of YANG nodes"
which is true for groupings and enumerations, but not so for identities...
Section 3.10.1:
The text says:
"When enumerating schema paths, a path to a list entry MUST include all key names in brackets."
Why?  I assume is for consistency between schema/data ypaths, but it's very different and seemingly unnecessary...
Note that, in the schema tree, a "list" is effectively the same as a "container"
The text says:
"A path that uses the key name as a child segment is invalid."
but what if the ypath wants to reference a key leaf?  e.g., in a "leafref" expression?
Section 3.10.3:
The text says:
"A keyless list in instance data is refered to the same way"
What about using a positional index for a list without keys, per RFC 7950 Section 9.13?
Section 3.14:
This section (along with 3.15 and 3.16) is where the text becomes very filter-specific
The text says:
"If any key in a multi-key list uses a wildcard, all keys in that list entry MUST use wildcards"
Why is this?
Section 4:
The formal syntax only defines an absolute path form, which means it could never be used in must/when statements in groupings.

Minor:
Section 1.1:
The text says:
"convey path information in management protocol error reporting"
but this is the same as "identify specific nodes in YANG instance data" and so seems unneeded.
The text says: 
"This document defines the ypath format only. It does not specify how ypaths are carried on the wire, stored, or processed by a particular protocol such as NETCONF [RFC6241] or RESTCONF [RFC8040]."
Fine, but should the text provide guidance as to if/must/which additional specifications are needed?  [See my comment above regarding an Appendix section]
Section 3.2:
Should "module:identifier" be "module-identifier"?
The text says:
"It is also valid to provide the module name on every path segment."
but maybe we shouldn't allow this? 
Section 3.8:
The text says:
"When walking a schema tree to enumerate paths, implementations MUST emit a path segment for both the choice and case nodes."
What about for shorthand "case" statements, pre RFC 7950 Section 7.9.2
Section 3.10.2:
The text says:
"Key values that are numeric or boolean MAY appear without quotes"
what about the "decimal64" type?
Section 3.13
This section regards Action nodes, but it is unclear when actions need to be addressed.  
Schema or instance tree?
And, if they do need to be addressed, wouldn't "rpc" statements also need to be addressed?  
...and then, what about "notification" statements? 
Section 3.17:
This section is so unexpected.  Consider removing.

Nits:
Section 1:
OLD: such as when and must statements
NEW: such as "when" and "must" statements
Section 1.1:
The text says:
"Ypath is a string syntax for identifying locations in a YANG data tree."
Not just data tree, but schema tree also, right?
Section 3.9:
OLD: /ietf-routing:routing/control-plane-protocols/control-plane-protocol[type]/type
NEW: /ietf-routing:routing/control-plane-protocols/control-plane-protocol[name]/type