Re: [Teas] Yangdoctors early review of draft-ietf-teas-yang-path-computation-11

"Belotti, Sergio (Nokia - IT/Vimercate)" <sergio.belotti@nokia.com> Mon, 08 February 2021 16:29 UTC

Return-Path: <sergio.belotti@nokia.com>
X-Original-To: teas@ietfa.amsl.com
Delivered-To: teas@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 734A83A1141; Mon, 8 Feb 2021 08:29:30 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.151
X-Spam-Level:
X-Spam-Status: No, score=-2.151 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.25, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, RCVD_IN_MSPIKE_H2=-0.001, SPF_PASS=-0.001, URIBL_BLOCKED=0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (1024-bit key) header.d=nokia.onmicrosoft.com
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 2ERu0T1L6WwE; Mon, 8 Feb 2021 08:29:28 -0800 (PST)
Received: from EUR02-HE1-obe.outbound.protection.outlook.com (mail-eopbgr10139.outbound.protection.outlook.com [40.107.1.139]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id C41B93A1107; Mon, 8 Feb 2021 08:29:24 -0800 (PST)
ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=OfXQh73+mz9FMUf41IU25+Ce81pVq1lEqgsV9/d2YN/Gck+uMwjWMLBgWA/RfIfnmljaLlotUZOVfNjUUg0CgOvXmY2Y2JndBgttUcwkc0Od+X8VTp3TuVujoeNhcms3d/oFz/4KqcVJdYgMnY7xKUo9QMmxEVIJnZyQM/AdzW2+yrO7/1fkCa/zbBwc8cBV/JvzVSkPTnm5ddz+XThZHDeilksgOwZ+Kwkds4DdAS6hwON4xb9m0lua3c63qDRfA1ejqxPNrR0uS5C7orNIBLHy9ZEGK5a4kYtDJNZZzqhJclu59Lq/D7Hmi4M/IVsWldqOLopRFf6xHGUpbdC1zA==
ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=cd+jy1wfgUCboYlnIPEJGEBwY5MMJ4hryw4yLlgxDEk=; b=NowVK0lLY6tdvVewPn1/RPNVmqG9EHRWqiHbhfEPdy0lJOgxI0uRASrO7mZWsbKnBgkXgiIjg4Na+a1KOJI/yYQR0X13Z1RdzoFVGUjOahbLgrOYPleny3+iPLvw3RnYV5W2PyfWeouaLSFA708eseho8oWi5en34QSUP+YVNiUZ9EI4pdqaC9caDV7gs/RyHw/OJpwgn8poxF4C/nz1KkKJL/2c6BNTNu37cDnSubbuok3CwtH0viw0oQHb2p8NGuQmksZj+H8NnLk2TbtnzWhefpaPXjPgov716dZIpaaut8iJjTOPLYCUK7r087TPvTzwwE/yALUSR+I0btUIHQ==
ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=nokia.com; dmarc=pass action=none header.from=nokia.com; dkim=pass header.d=nokia.com; arc=none
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nokia.onmicrosoft.com; s=selector1-nokia-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=cd+jy1wfgUCboYlnIPEJGEBwY5MMJ4hryw4yLlgxDEk=; b=NEcVuq8rXMGZUnbimz2MJeubITjtFD0e4kPH/06kFqswli89xmIzlbqxMQg+jXnaLz9CrXBKFBYmC8Yh0rQYE7RdUlYA7YFmjMaNJvkrHZalKAZDYm3nGh+3ToMLV2qgRZ+Bw8tWTnWXkoYLcbFBvK4zLSFi4R7cmGs+OGK9sEE=
Received: from (2603:10a6:208:104::27) by AM8PR07MB7345.eurprd07.prod.outlook.com (2603:10a6:20b:249::13) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3825.10; Mon, 8 Feb 2021 16:29:21 +0000
Received: from AM0PR07MB5490.eurprd07.prod.outlook.com ([fe80::edbd:cb31:170a:f3d6]) by AM0PR07MB5490.eurprd07.prod.outlook.com ([fe80::edbd:cb31:170a:f3d6%4]) with mapi id 15.20.3846.025; Mon, 8 Feb 2021 16:29:21 +0000
From: "Belotti, Sergio (Nokia - IT/Vimercate)" <sergio.belotti@nokia.com>
To: =?utf-8?B?TWFydGluIEJqw7Zya2x1bmQ=?= <mbj+ietf@4668.se>, "yang-doctors@ietf.org" <yang-doctors@ietf.org>
CC: "draft-ietf-teas-yang-path-computation.all@ietf.org" <draft-ietf-teas-yang-path-computation.all@ietf.org>, "teas@ietf.org" <teas@ietf.org>
Thread-Topic: Yangdoctors early review of draft-ietf-teas-yang-path-computation-11
Thread-Index: AQHW1Kal3KLWdyJxsEWl4pd0iayHu6pOwWTA
Date: Mon, 8 Feb 2021 16:29:21 +0000
Message-ID: <AM0PR07MB5490C0C5C95A5F7333C8B942918F9@AM0PR07MB5490.eurprd07.prod.outlook.com>
References: <160823152245.15025.13731491639863650578@ietfa.amsl.com>
In-Reply-To: <160823152245.15025.13731491639863650578@ietfa.amsl.com>
Accept-Language: it-IT, en-US
Content-Language: en-US
X-MS-Has-Attach:
X-MS-TNEF-Correlator:
authentication-results: 4668.se; dkim=none (message not signed) header.d=none;4668.se; dmarc=none action=none header.from=nokia.com;
x-originating-ip: [79.16.86.188]
x-ms-publictraffictype: Email
x-ms-office365-filtering-ht: Tenant
x-ms-office365-filtering-correlation-id: 2e85d647-03ff-4dd9-4f1d-08d8cc4eb07a
x-ms-traffictypediagnostic: AM8PR07MB7345:
x-microsoft-antispam-prvs: <AM8PR07MB734540F791856F75075F82AD918F9@AM8PR07MB7345.eurprd07.prod.outlook.com>
x-ms-oob-tlc-oobclassifiers: OLM:10000;
x-ms-exchange-senderadcheck: 1
x-microsoft-antispam: BCL:0;
x-microsoft-antispam-message-info: lqYpMgRwv1gBbjQKWPQwAgAyPkiOItqD/8Us8T0zerqBwb3aj5xe0rW8Ek31mUIx80OJnRT/c++u2PUPRABGLm+63gS/iiBu/yU3Q9gGnOIivrHXvi9LBeVxeklu1IIgNd6faJ/Sbqd1MLPDEcmfZHUwKkGpaFvW+lDJsroHvGHT+ZOBqMB5Ia7h2OdL/yjT88pXs05sd+FGuAWZ/1x3HRH61qt7HRUcKatyOyQ2o2IKSXB8Kirebntw9HniqLjdaJT8g6HUOCFlMno0BzMKHTxbeHM7xzoodqtiIwkdRsP1jzIk7qrIYnqDYnVL2kLxOpZQVNKWz2xgohGS1f8BGGNYJCNM4OjeR6YeIqmqZDBhKjecBlTov8E2bIgkB8ajPVgyMy3Q70nmGb4fSelVMJGkpn6V6Lc5gtoEsXOAC/3p9yDuJ9InRto6OqSzldsabifG90ydecl1AkDw2+6Le8ruzHrDzg0094XIEWi5zODMciPi3R2GFQgqCgQ9CIu8bA8YK7+z4+p0I6nmJXqFFFpKCgBa4AeBXQSJIFptM+affd0SIg/lHhgjwa6F0JJEEWytkj9kMOFSa6sQDdiAzbU1n5tOqBWv0J6XdYdUhGU=
x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:AM0PR07MB5490.eurprd07.prod.outlook.com; PTR:; CAT:NONE; SFS:(4636009)(396003)(376002)(346002)(39860400002)(366004)(136003)(2906002)(66574015)(6506007)(4326008)(53546011)(26005)(5660300002)(316002)(66556008)(66946007)(64756008)(8676002)(8936002)(186003)(966005)(83380400001)(478600001)(9686003)(71200400001)(7696005)(33656002)(54906003)(110136005)(55016002)(86362001)(76116006)(52536014)(66446008)(66476007); DIR:OUT; SFP:1102;
x-ms-exchange-antispam-messagedata: =?utf-8?B?Qlh1TDdzSk4rNURvdTFPSEZyTXVOOUZSVzNwQ0UvdFVkVVBMd00xSmNjbVBO?= =?utf-8?B?NEV0NC9kYlpRZWhIb1QwSVlFakpKdWhIYzhPc0djNjVXQ0FBdngxb0c5aHRE?= =?utf-8?B?cDN6QUdSVTI4ZWFXSVA5QzdHbHZxSGI3Mzk1RDVCVVlsRW44ZTd4YWI2Q2t0?= =?utf-8?B?bWxFeklxbnRDQlgyMVV4ZXMvMmJFQUtYaVYwZmFPQ0VXT1hQRDZQY2pPYmx4?= =?utf-8?B?K2NUY2UrQ2w3NjRiMmp2SjZrSXRVeUJEdkNqLzJuSVM2Q2dkUS82V0tHaVFt?= =?utf-8?B?S2JHcjR4UERPREFBRllrS1kxMjdNVmRDY24veEZFcDNTU01scndRUkFYYkxq?= =?utf-8?B?VGxyNi9SM2J3WmJEcmpHeC9KdWI3eU5yWmhDTkt4end3c0x2anp4TDFteHFn?= =?utf-8?B?VSszeTF1L292YjFBWTZwWHNzRVl5Tk0xSnpnclFnZXdRa2R2d3VCNjllU050?= =?utf-8?B?OWcxYmdkQ21hbmNNK0hQSEUzZVFMdnFTRVAvdUhJRnMvM1l0eTdCQW1mVnZO?= =?utf-8?B?U1krTW1RcEpGSmx6LzgrRVNBeVhJOEZ2bHZxMlU2MmM4cnFxZ0xGK2VFdXhK?= =?utf-8?B?ZWVCRkt2S2htdFk0bkVvWTY2ekM3OWpBdXkwYW9HaU1VVi9CaDRiQkxneUVx?= =?utf-8?B?ZFNBNFVzVmg0UnZ0S2Z1eXgxUlJrTDZUMEhqaGIxbnFRRnQ0Q3djZFhtRUU3?= =?utf-8?B?TndYTmI0dGpwOUVuWmVjdUxwZVRzRk5IUFZpd0g1M0QyQ1A1MENHWm5teWJO?= =?utf-8?B?R01rZmNkSzdqTWpTL0xRVWNCUEpQM3ZIMjJZTlFwblR6b2JaSFZNWkZ1TzlV?= =?utf-8?B?QnRnNlozZC9JRHZvaDlkMXBwUzJmTndQT1I1WTFoTXhSdjZWK2VsQ3kwa0g2?= =?utf-8?B?aTl1ekRJeHJ2OVVadG00N25zNHkzQmc3dFQ5c09udmZidXdMNGRwUzk2R1Zv?= =?utf-8?B?SGUvRDNZVUliakZMRFFDZWJTUEI4ZWtOSWp0QXJCVzdmRFFJYlJMWVRYNkVz?= =?utf-8?B?WWpoK3Y4N1ZBeHZNR0JVeFF1L3NJZEQvb3drbzNKa052b2ZtNjdZeHoyOTFI?= =?utf-8?B?VWhYY3dXaTgrRld4WVVxYU9qa2M3S3cvYnJkOE9Od2V1SXlTek95a2tDelFU?= =?utf-8?B?L0xiRlNrc2RiMm1kTSsyRG9DWWdDRXlTVzlBV2wzS3FhczdhRFRMcGxlWWFO?= =?utf-8?B?MlQ4ZnI0dnJNMGpxcUVLTW5sM3F3ZWJpczFiUmNDaS96ZnR6dDRkckMrbjZU?= =?utf-8?B?SnBHNXdTMlVjU3pvZWdZaWNGNkhleUNyY0hLWS9EenhZS2VNV2QyT0YzS1FC?= =?utf-8?B?WEVoY1E2M0V6Tk9vYjZrUnhkZWxIc011cVMwdjk0STIxUjVjTnNLRFZCTzNo?= =?utf-8?B?NkhpS2xBRnlUdVMxMFBmamVjb3VNRlNqUmdPSUtzR3hmRFpsbG5DZXJUM25l?= =?utf-8?B?elJtRDU5K1FmZENlaWh0alMzREVLdUNVandQR3F4Q2psOU1RazNjS0VUeUUv?= =?utf-8?B?SCtTWGhDR2NQbUxYWXgrV21vSFI1S0o3Sm1PRG1NbjJJRFVJMzdpSWJ3bFJL?= =?utf-8?B?SkkzWGlrSVRpU3l2R2diandhekhqWFBKdTRaWXhlWWFJb1Z3T2VHUGtkNnlD?= =?utf-8?B?SXlnWUMvaGdFY0UrYkZ0VU1hREdNK1dRWTNpT1krdEJVVzIxT2R3aVY3djly?= =?utf-8?B?dGh1SG41Q3oxS09nemp0cTN2RVNCY1krMHB0Nk8yTDQwUDlMQjA0R2s0YUlQ?= =?utf-8?Q?4qbDstMR6FAthtJs+c=3D?=
x-ms-exchange-transport-forked: True
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: base64
MIME-Version: 1.0
X-OriginatorOrg: nokia.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: AM0PR07MB5490.eurprd07.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 2e85d647-03ff-4dd9-4f1d-08d8cc4eb07a
X-MS-Exchange-CrossTenant-originalarrivaltime: 08 Feb 2021 16:29:21.1687 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 5d471751-9675-428d-917b-70f44f9630b0
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: Ukh28m7XlxH/SWf3DZKCJR+vmDoBoBnWD5ENR6oQVSBrlfXO2dbBE0PRyjR2vOBS+eAApBW431ccIsfW9qS55FTBnHg4F2OBUXuUcNUq548=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM8PR07MB7345
Archived-At: <https://mailarchive.ietf.org/arch/msg/teas/IDMfvdeAttFvrJap8TXEHUA2ds0>
Subject: Re: [Teas] Yangdoctors early review of draft-ietf-teas-yang-path-computation-11
X-BeenThere: teas@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Traffic Engineering Architecture and Signaling working group discussion list <teas.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/teas>, <mailto:teas-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/teas/>
List-Post: <mailto:teas@ietf.org>
List-Help: <mailto:teas-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/teas>, <mailto:teas-request@ietf.org?subject=subscribe>
X-List-Received-Date: Mon, 08 Feb 2021 16:29:31 -0000

Hi Martin,
Thanks for your review, we have addressed them in the version-12 we have just submitted.
See our feedbacks and some follow up questions in line marked with "Authors>".

Thanks
Italo and Sergio

> -----Original Message-----
> From: Martin Björklund via Datatracker <noreply@ietf.org>
> Sent: Thursday, December 17, 2020 7:59 PM
> To: yang-doctors@ietf.org
> Cc: draft-ietf-teas-yang-path-computation.all@ietf.org; teas@ietf.org
> Subject: Yangdoctors early review of draft-ietf-teas-yang-path-computation-
> 11
> 
> Reviewer: Martin Björklund
> Review result: Ready with Nits
> 
> 
> o  General
> 
>   The language is called "YANG", not "Yang".

Authors> ok

> 
> 
> o  1.2. Tree Diagram
> 
>   The text says:
> 
>     A simplified graphical representation of the data model is used in
>     section 6.1 of this this document.  The meaning of the symbols in
>     these diagrams is defined in [RFC8340].
> 
>   Tree diagrams are used also in chapter 5.  I suggest:
> 
>     Tree diagrams used in this document follow the notation defined in
>     [RFC8340].

Authors> ok

> 
> 
> o  Tree diagrams in general
> 
>    You can use pyang -f tree --tree-line-length 68 ... in order to
>    avoid long lines in the RFC.

Authors> we are using the same pyang string but with 69 limit, but it seems not always (or for all lines) working. Why 68 and not 69? Why something should change?

> 
> 
> o  6.1
> 
>   This section presents a fully expanded tree diagram of the module.
>   Tree diagrams are mainly used to give an overview of a module's
>   structure.  The tree diagram in this section spans 13 pages and is
>   quite hard to read.
> 
>   I also note that a majority of the nodes in this tree diagram come
>   from the expansion of groupings that aren't defined in this
>   document.  Hence, I suggest that you might want to run:
> 
>     pyang -f tree --tree-line-length 68 \
>       --tree-print-groupings --tree-no-expand-uses
> 

Authors> we have tried but it seems that also the groupings defined in the module are not expanded and we think at least these groupings should be expanded in the tree.

> 
> o  There are a number of groupings that are used only once, and do not
>    seem to be defined to be reused by other modules, e.g.,
>    "requested-info", "requested-state", "svec-metrics-bounds" and more.
> 
>    If they are intended to be reused, it should be made clear in their
>    description statements.  If not, I think they should be inlined and
>    removed.

Authors> We think that it is sometimes necessary to introduce groupings even if they are not re-used to make the YANG code structured and readable. This is the same logic in other programming, where "helper" functions are used to code complex logics.

Authors> Following this approach we have kept the following groupings:

- requested-info
- requested-state (moving the container statement inside)
- reported-state
- synchronization-constraints
- synchronization-optimization
- synchronization-info

and removed the following groupings:

- svec-metrics-bounds
- svec-metrics-optimization
- svec-exclude

> 
> 
> o  grouping svec-exclude
> 
>    This grouping has an ordered-by user list.  Why is this list user
>    ordered?  If the order matters, it should be explained how it matters.

Authors> you're right. Deleted "ordered-by user list". No reason for that.

> 
>    Also, the index leaf has this description:
> 
>      "XRO subobject index"
> 
>    What is "XRO"?  Is this description sufficiently clear?

Authors> We have removed the index since lists in RPC input do not need it.

> 
> 
> o  path-request
> 
>    In the path-request, there is construct for path-refs:
> 
>                    list primary-reverse-path-ref {
>                      key index;
>                      min-elements 1;
>                      description
>                        "The list of primary reverse paths that
>                         reference this path as a candidate
>                         secondary reverse path";
>                      leaf index {
>                        type uint32;
>                        description
>                          "The index used by the
>                           primary-reverse-path-ref list";
>                      }
> 
> 
>   What is this index?  Is it only used as an arbitrary index, or
>   something else?  If it is an arbitraty index, it should be explained
>   in the descriptions.
> 
>   Also note that lists in rpc input don't need an index.

Authors> OK, deleted. We have removed index both in "primary reverse path ref" and "primary path ref".

> 
> 
> o  Validation
> 
>    The module fails YANG validation, but that is really due to errors
>    in ietf-te@2020-07-12.yang.  Specifially, the leafref in the
>    grouping "path-compute-info" must have prefixes in its path.
>    Without prefixes, the path refers to nodes in the module that uses
>    the grouping.  (same for other groupings in that module).

Authors> See https://github.com/tsaad-dev/te/issues/114


> 
> 
> o  Layout
> 
>   I suggest you run the module through
> 
>     pyang -f yang --yang-canonical --yang-line-length 68
> 
>   in order to have the module indented and formatted consistently.
> 
>   This will make the RFC editor's job easier.

Authors> We are using " pyang --strict --ietf --max-line-length 69 -f tree --tree-line-length=69 -o ietf-te-path-computation.tree ietf-te-path-computation.yang". Is something wrong with this string command?

> 
> 
> 
> /martin
> 
>