Hi Balazs,

To follow up with our conversation earlier.  Andy and Juergen explicitly copied 
because they may have previously commented on these issues during WG LC.

I think that my comment regarding the "feature statement" and the flexibility 
of the inline-method are closely related.  I find the definition of the 
inline-content-schema to be so generic that it effectively allows anything.  
E.g., the drafts would allow me to publish a file that has an 
inline-content-schema based on robs-random-schema-format@1.0.0, and it would be 
very difficult for consumers of the associated instance data file to understand 
the file schema.

Similarly, I find that allowing revision labels (as examples to avoid a 
normative reference to the module versioning draft), makes it hard for a 
generic implementation reader of a instance data file to know how to interpret 
an inline schema.  I suspect that this issue could cause problems in the IESG 
reviews.

Hence, my preference, for this RFC, that defines version 1 of the instance file 
format, would be to more heavily constrain how the schema is allowed to be 
specified in the inline-method.  Specifically, I think that it would be better 
to:
 - restrict the inline schema to only be defined using 
ietf-yang-library@2019-01-04
 - only allow revision-dates, not revision labels.

I would like to understand from Andy, whether he still thinks with these 
restrictions whether the inline-schema method should still be under a YANG 
feature statement?

If/when the revision labels draft gets standardized, and perhaps also after 
YANG packages, then we could do a bis version of this document to define a v2 
of the instance file format that potentially allows YANG packages to be used to 
define the schema, and potentially allows modules to be identified using 
revision labels as well as revision dates.

Balazs, I'm good with most of your proposed resolutions, but have answered one 
further question inline below.


> -----Original Message-----
> From: Balázs Lengyel <balazs.leng...@ericsson.com>
> Sent: 05 July 2021 13:47
> To: 'netmod@ietf.org' <netmod@ietf.org>; Rob Wilton (rwilton)
> <rwil...@cisco.com>
> Cc: Benoit Claise <benoit.cla...@huawei.com>
> Subject: FW: AD review of draft-ietf-netmod-yang-instance-file-format
> 
> Hello Rob,
> Thanks for the review.  Here are my answers below. I will also upload the
> new version asap.
> Regards Balazs
> -----------------------------------------------------------
> Hi,
> 
> Here is my AD review of draft-ietf-netmod-yang-instance-file-format-13.
> 
> Thanks for this document, I think that it represents important useful work
> for advancing the YANG ecosystem.
> 
> This document is in good shape, and I mostly have minor comments but with
> a
> few more significant comments.
> 
> Main comments:
> 

> 
> 2.
> In the YANG Module:
>      feature inline-content-schema {
>        description
>          "This feature indicates that inline content-schema
>           option is supported. Support for this feature might
>           be documented only via out-of-band documentation.";
>      }
> 
> What is the benefit of having 'inline-content-schema' as a feature?  It
> seems to potentially add complexity without any benefit, given that the
> device originating the instance data file would effectively choose whether
> to use the inline-content-schema, hence I suggest that it might be simpler
> just to remove the feature definition.
> BALAZS: This was explicitly requested earlier by a reviewer (Andy ?).
> The system can declare supported/not-supported in design documentation.
> In a use-case when a client or a design department is sending data to a
> server this is needed. E.g. in UC2, Preloading Default Configuration the
> designer preparing instance data, can decide to use or not use the
> inline-content-schema based on this.


> 
> 3.
> In the YANG Module:
> 
>       "case inline", description:
>                     The first item is either ietf-yang-library or
>                     some other YANG module that contains a list of
>                     YANG modules with their name, revision-date,
>                     supported-features, and deviations.
>                     The usage of revision '2019-01-04' of the
>                     'ietf-yang-library' module MUST be supported.
>                     Using other modules, module versions MAY also
>                     be supported.
> 
> This seems to make interop for consumers of instance data files hard, since
> the schema can be defined by any arbitrary YANG module without updating
> this
> module.  I would suggest that it is safer to limit this to the two currently
> published versions of YANG library.
> BALAZS:  I fully agree, however this was explicitly requested by some
> reviewer earlier (Juergen ?) Shall I simplify this or not?
> 
> If additional modules are supported in future, then I think that it would be
> safer to create a new version of this YANG module that documents what
> other
> module formats can be used.
> 
> 
> 4.
> In the YANG Module:
>       list "revision"
> 
> Is revision expected to be unique, if provided? If so, should this be
> explicitly stated in the YANG module description?
> BALAZS: I don't think I understand your comment. There may be multiple list
> entries for revision. The 'leaf date' is a key, so it is inherently unique.
> The description may or may not be unique.

I would suggest changing:

For every published editorial change, a new one SHOULD be added
 in front of the revisions sequence so that all

to:

For every published editorial change, a new unique revision SHOULD
be added in front of the revisions sequence so that all

I.e., to also make it clear in the description that revisions dates are 
required to be unique.


> 
> 
> 5.
> In the YANG Module:
> 
> Is an instance-data file allowed to contain both a revision and also a
> timestamp?  If so, is there any constraints on the values.  If not, then
> would it make sense to put them under a choice?
> BALAZS:  It is allowed to have both. There is some recommendation text
> about
> when to use each. However I can see some corner cases, when using both in
> the same file would be useful, E.g. we want a timestamp including hour,
> minutes, but we also want the history of the instance data set, including
> multible revision/descriptions.
> I propose to add: if both are included the timestamp, SHOULD contain
> the same date as the latest revision statement.

Okay.

Thanks,
Rob

_______________________________________________
netmod mailing list
netmod@ietf.org
https://www.ietf.org/mailman/listinfo/netmod

Reply via email to