Martin Pels <[email protected]> writes:
> Hi Ladislav,
>
> I have posted a version 05 draft, addressing your comments. The known
> implementations have been updated to use the current model version.
>
> For the regular expressions: Using \c is not suitable, because this
> character class is only supported by XML regular expression flavours.
Well, YANG uses the XSD regular expression flavour (sec. 9.4.5 in RFC 7950),
but it is in no way limited to XML.
> Definition files may also be specified as JSON, and these can then not
> be validated properly. I did simplify the regexes, because the yang-char
Every YANG validator compliant to RFC 7950 has to be able to validate strings
against XSD regex patterns, no matter what the instance data representation is.
Lada
> syntax defined in RFC7950 already excludes control characters.
>
> Kind regards,
> Martin
>
> On 21/05/2025 11:46, Ladislav Lhotka via Datatracker wrote:
>> 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.
>>
>>
>>
>> _______________________________________________
>> GROW mailing list -- [email protected]
>> To unsubscribe send an email to [email protected]
>
--
Ladislav Lhotka <[email protected]>
PGP Key ID: 0xB8F92B08A9F76C67
_______________________________________________
GROW mailing list -- [email protected]
To unsubscribe send an email to [email protected]
