Re: [Teas] Yangdoctors early review of draft-ietf-teas-yang-te-34

[Tarek Saad <[e-mail]>]

Hi Andy,

Thank you for the review and comments. We have uploaded rev 35<https://datatracker.ietf.org/doc/html/draft-ietf-teas-yang-te-35> that addresses comments you have raised. Please see inline for more details [TS].


From: Andy Bierman via Datatracker <[e-mail]>
Date: Tuesday, November 28, 2023 at 3:15 PM
To: [e-mail] <[e-mail]>
Cc: [e-mail] <[e-mail]>, [e-mail] <[e-mail]>
Subject: Yangdoctors early review of draft-ietf-teas-yang-te-34
Reviewer: Andy Bierman
Review result: Ready with Issues



# ietf-te.yang

## Summary

-  Well written large module representing a lot of work.
-  No YANG errors or warnings.

## Major Issues

-  None

## Minor Issues

### leaf hierarchy/dependency-tunnels/dependency-tunnel/name

-  The 'tunnel-ref' typedef is duplicated here.

OLD:

            type leafref {
              path "/te:te/te:tunnels/te:tunnel/te:name";
              require-instance false;
            }

SUGGESTED NEW:

            type tunnel-ref {
              require-instance false;
            }

[TS]: accepted.


### list lsp-provisioning-error-infos/lsp-provisioning-error-info

-  This list has no key leaf(s) defined. It is used in 4 places.
   Although legal, this limits list entry retrieval options

[TS]: we discussed this amongst authors. We found no strong case to index the items in this read-only list. So, we opted to keep as-is.


-  The description (or reference) should explain when/how a server is
   expected to add and remove entries from this list.

-  Also applies to /computed-path-error-infos/computed-path-error-info
[TS]: these entries are populated by the backend path computing server and are “read-only”. Hence, those entries are not expected to be removed. We have expanded description of this in section 5.1.2.

### action tunnel-action

-  description-stmt should explain the purpose of this action
[TS]: updated description to describe the purpose.

-  reference-stmt should be added, if applicable
[TS]: updated.


### action protection-external-commands

-  no description-stmt. It should explain the purpose of this action

-  reference-stmt should be added, if applicable
[TS]: updated.


### rpc tunnels-path-compute

-  description-stmt should explain the purpose of this operation.
   It looks like this operation is just a generic API,
   and the purpose, input, and output parameters are all
   added by augment.
[TS]: updated description to describe the purpose.

-  reference-stmt should be added, if applicable
[TS]: updated.


### rpc tunnels-actions

-  description-stmt should explain the purpose of this operation.

-  reference-stmt should be added, if applicable
[TS]: updated.


## Nits

-  General: action and RPC naming is not very consistent.

-  pg 14: the 'tunnels' list

   The YANG list node is named 'tunnel'.
[TS]: fixed.

# ietf-te-device

## Summary

-  Well written large module representing a lot of work.

-  No YANG errors or warnings.

## Major Issues

-  None

## Minor Issues

### interface/interface

-  list interface with key leaf interface is confusing.
   Descriptions for both should be expanded.
[TS]: addressed by renaming the key to name.

Regards,
Tarek (for the co-authors)


## Nits

-  None