Hi Med,

[email protected] writes:

> Hi Lada, 
>
> Thanks for the review. As you rightfully noticed:
>
>> The ietf-bgp-communities module represents an interesting use case
>> where YANG is used outside of its standard server-client domain for
>> publishing information about configured BGP communities. YANG
>> essentially plays a role of a (JSON) schema language.
>
> The companion question is whether following RFC8791 makes more sense here or 
> keep the current design.
>
> How do you feel about this?

I think it can stay as it is. As I wrote in the review, it is IMO perfectly 
possible that the BGP community data might eventually be available from a 
RESTCONF server.

Actually, I am not a fan of the structure extension. It would be much cleaner 
to restructure YANG definition and move the NETCONF-specific parts to a 
separate document. A specification that defines YANG as a multi-representation 
schema language would also make it much more accessible to potential users of 
YANG outside of the NETCONF domain - as it is in the present case.

Lada

>
> Cheers,
> Med
>
>> -----Message d'origine-----
>> De : Ladislav Lhotka via Datatracker <[email protected]>
>> Envoyé : mercredi 21 mai 2025 11:46
>> À : [email protected]
>> Cc : [email protected];
>> [email protected]
>> Objet : draft-ietf-grow-yang-bgp-communities-04 early Yangdoctors
>> review
>> 
>> 
>> Document: draft-ietf-grow-yang-bgp-communities
>> Title: A YANG Data Model for BGP Communities
>> Reviewer: Ladislav Lhotka
>> Review result: Ready with Nits
>> 
>> The ietf-bgp-communities module represents an interesting use case
>> where YANG is used outside of its standard server-client domain for
>> publishing information about configured BGP communities. YANG
>> essentially plays a role of a (JSON) schema language.
>> 
>> **** General comments
>> 
>> - Section 6 (Data Elements) contains a hierarchy of subsections,
>> one for each data node of the schema tree. These subsections
>> (i) repeat information that is formally specified in the YANG
>> module, and (ii) sometimes add extra explanations. Unless there is
>> a good reason for having this section in the document, I'd suggest
>> to remove it. Duplicated information invites troubles if this
>> section and the YANG module get out of sync, and the extra
>> information, if it is substantial, should be added to descriptions
>> in the YANG module.
>> 
>> - Running code seems to exist, which is always good, and the
>> document refers to two autonomous systems that already use the
>> schema defined in the YANG module (well, almost, see below). In
>> both cases, only an entire read-only JSON file can be retrieved
>> from an URL. I think it might be useful to consider a more granular
>> service, e.g. using a RESTCONF server, and perhaps even a
>> possibility of remote edits.
>> 
>> **** Specific comments
>> 
>> - The two examples of BGP community data published by AS197000 and
>> AS25152 are based on an old revision of the YANG module – words in
>> multi-word identifiers, such as "email-address" are now separated
>> by a hyphen. Both JSON files also contain other trivial errors:
>> 
>>   * namespace identifiers should be "ietf-bgp-communities" rather
>> than
>>   "draft-ietf-grow-yang-bgp-communities", see RFC 7951
>> 
>>   * lists "contacts" and "fields" shoud in fact be "contact" an
>> "field".
>> 
>> - At several places, regular expressions are used for specifying
>> the set of permitted Unicode characters. For example, the
>> definition of "community-description" typedef contains the
>> following statement:
>> 
>>   pattern '[^\p{C}]+';
>> 
>>   I am not sure that this matches the intent – are parsers expected
>> to handle
>>   combining and extender characters? If so, a positive definition
>> using the
>>   "\c" character class (XML 1.0 name characters) might be
>> preferable.
>> 
>> - Section 8.1 instructs operators to order overlapping community
>> definitions from most to least preferred. Shouldn't then the
>> "regular", "extended" and "large" lists be defined as "ordered-by
>> user"? Alternatively, a "preference"
>> leaf could be added to make this explicit.
>> 
>> - In Appendix A, it would be useful to provide an example of
>> extended community definition.
>> 
>> 
>
> ____________________________________________________________________________________________________________
> 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.

-- 
Ladislav Lhotka <[email protected]>
PGP Key ID: 0xB8F92B08A9F76C67

_______________________________________________
GROW mailing list -- [email protected]
To unsubscribe send an email to [email protected]

Reply via email to