Subject: [Last-Call] Yangdoctors last call review of draft-ietf-dots-rfc8782-bis-02

From: Ebben Aries via Datatracker <[email-masked]>
Date: Tue, 01 Dec 2020 15:22:10 -0800

Reviewer: Ebben Aries
Review result: Ready

2 modules in this draft:
- [email-masked]
- [email-masked]

YANG compiler errors or warnings (pyang 2.4.0, yanglint 1.9.2)
- No compiler errors or warnings however pyang 2.4.0 is currently the only
  compiler verified to emit YANG sx:structure data in tree format.  Instance
  data could not yet easily be validated given the current linters/compilers.

Links to previous relevant reviews:

-
https://datatracker.ietf.org/doc/review-ietf-dots-rfc8782-bis-00-yangdoctors-early-aries-2020-09-23/
-
https://datatracker.ietf.org/doc/review-ietf-dots-telemetry-14-yangdoctors-lc-lindblad-2020-11-26/

Updates/comments since prior review
------------------------------------

While there is a -02 version of the draft posted since the YANG Doctor review
request against -01, the updates reflect textual wording of the draft and YANG
module contents have not changed thus this review encompasses the latest -02
version as well.

Since the -00 version, the following previous review comments have now been
addressed:

- Textual wording in the draft around IPv4/IPv6 prefix lengths
- The prefix for 'ietf-dots-signal-channel' is now updated to a more
  descriptive 'dots-signal'
- Import prefixes are updated to reflect usage of the imported module prefix
- Author email address corrections
- The prefix for 'iana-dots-signal-channel' is now updated to a more
  descriptive 'iana-dots-signal' and thus this module now carries with it a
  revision bump

Updated nodes:

The 'lifetime' node has now been updated from a single 'int32' value to a
union of uint32 and int32 with a range restriction.  From a modeling
standpoint for the uint32 value, while '0' is invalid in a request, it appears
the status retrieval could very well still report '0' for a brief period of
time thus assume this is the reason for not implementing a range restriction
on the uint32 value as well indicating "1..4294967295"?

Now, when this translates into CBOR representation, the wire protocol I will
assume (Sorry just briefly researched this re: CBOR) will differentiate 2
separate types here so we don't run into -1 and 4294967295 both representing
indefinite lifetime?

While I share similar opinions to Jan's review in
review-ietf-dots-telemetry-14-yangdoctors-lc-lindblad-2020-11-26 in relation
to the draft and data structure definitions, I believe from a YANG doctor
review standpoint, we are ready to move forward with this draft/module-set.

---

From: <[email-masked]>
Date: Wed, 2 Dec 2020 06:27:17 +0000

Hi Ebben, 

Thanks for the review. 

Please see inline. 

Cheers,
Med

> -----Message d'origine-----
> De : Ebben Aries via Datatracker [mailto:[email-masked]]
> Envoyé : mercredi 2 décembre 2020 00:22
> À : [email-masked]
> Cc : [email-masked]; [email-masked]; last-
> [email-masked]
> Objet : Yangdoctors last call review of draft-ietf-dots-rfc8782-bis-
> 02
> 
> Reviewer: Ebben Aries
> Review result: Ready
> 
> 2 modules in this draft:
> While there is a -02 version of the draft posted since the YANG
> Doctor review request against -01, the updates reflect textual
> wording of the draft and YANG module contents have not changed thus
> this review encompasses the latest -02 version as well.

[Med] I confirm. 

> The 'lifetime' node has now been updated from a single 'int32' value
> to a union of uint32 and int32 with a range restriction.  From a
> modeling standpoint for the uint32 value, while '0' is invalid in a
> request, it appears the status retrieval could very well still
> report '0' for a brief period of time thus assume this is the reason
> for not implementing a range restriction on the uint32 value as well
> indicating "1..4294967295"?

[Med] Yes. 

> 
> Now, when this translates into CBOR representation, the wire
> protocol I will assume (Sorry just briefly researched this re: CBOR)
> will differentiate 2 separate types here so we don't run into -1 and
> 4294967295 both representing indefinite lifetime?

[Med] Whether this is an indefinite lifetime is determined from the CBOR major type (1 in this case for CBOR key 14 (lifetime)).    

> review-ietf-dots-telemetry-14-yangdoctors-lc-lindblad-2020-11-26 in
> relation to the draft and data structure definitions, I believe from
> a YANG doctor review standpoint, we are ready to move forward with
> this draft/module-set.

[Med] Thank you.

_________________________________________________________________________________________________________________________

Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.