Early Review of draft-ietf-teas-actn-vn-yang-10
review-ietf-teas-actn-vn-yang-10-yangdoctors-early-bierman-2020-12-20-00
| Request | Review of | draft-ietf-teas-actn-vn-yang-10 |
|---|---|---|
| Requested revision | 10 (document currently at 14) | |
| Type | Early Review | |
| Team | YANG Doctors (yangdoctors) | |
| Deadline | 2020-12-18 | |
| Requested | 2020-11-13 | |
| Requested by | Vishnu Pavan Beeram | |
| Authors | Young Lee , Dhruv Dhody , Daniele Ceccarelli , Igor Bryskin , Bin-Yeong Yoon | |
| Draft last updated | 2020-12-20 | |
| Completed reviews |
Yangdoctors Early review of -10
by
Andy Bierman
(diff)
|
|
| Assignment | Reviewer | Andy Bierman |
| State | Completed | |
| Review |
review-ietf-teas-actn-vn-yang-10-yangdoctors-early-bierman-2020-12-20
|
|
| Posted at | https://mailarchive.ietf.org/arch/msg/yang-doctors/Jc7RQe0KruqarQacH23IBnCuzyg | |
| Reviewed revision | 10 (document currently at 14) | |
| Result | Ready with Issues | |
| Completed | 2020-12-20 |
review-ietf-teas-actn-vn-yang-10-yangdoctors-early-bierman-2020-12-20-00
Major Issues:
None
Moderate Issues:
1)
leaf /ap/access-point-list/access-point-id
leaf /vn/vn-list/vn-id
leaf /vn/vn-list/vn-member-list/vn-member-id
These list keys use type inet:uri.
You should consider the implementation complexity here.
Will servers all correctly convert any arbitrary URI to its canonical
representation? The draft should address this issue.
2)
leaf /vn/vn-list/oper-status
leaf /vn/vn-list/admin-status
These objects use vn-status-type, vn-admin-state-type
The use of identities for even simple "up/down" status types
seems extreme. The conformance for an enumeration is clear
(mandatory), but not for an identityref type.
- E.g., Is is mandatory for a vendor to support vn-state-up,down?
A vendor could write their own identities and ignore the standard
identities.
3)
rpc /vn-compute
The procedure for this operation is not explained here.
A full description or reference to normative test is needed.
- what does the server do with the input?
- what output is expected? Any variants based on the inputs
should be explained.
- any interoperability considerations wrt/ use of these
common groupings in this RPC context?
- what errors can occur? Specify any error-tags, etc. that
the server MUST/SHOULD include in the response
Minor Issues:
4)
- naming inconsistent within /ap and /vn
access-point is spelled out and vn is not
Suggest: shorten access-point to ap
5)
- naming a list entry with the the suffix -list is redundant.
YANG lists do not have any conceptual container or way to
reference all the entries (if that what this naming intends)
Suggest:
s/access-point-list/ap/
s/vn-list/vn/
s/vn-member-list/vn-member/
6)
- leaf /vn/vn-list/vn-member-list/src/multi-src
- leaf /vn/vn-list/vn-member-list/dest/multi-dest
Looks like these leafs should each have a default-stmt.
What does it mean if the multi-src-dest feature is supported
but these leafs are missing from the config?