Subject: [netmod] kw review of draft-ietf-netmod-yang-instance-file-format-02

From: Kent Watsen <[email-masked]>
Date: Mon, 25 Mar 2019 07:03:15 +0000

The Abstract says “re-using the same format as the reply to a <get> operation/request”.  At first, I was going to suggest replacing <get> with <get-data>, but I question if this is correct since the draft supports JSON, which neither NETCONF RPC is able to return. 

The Terminology section is missing an entry for “instance data set”. 

I don’t understand what P3 means.  Add info text to draft.

Re: P4, if file == 1 “set”, then the distinction becomes unclear (see earlier comment about missing “set” term).  I understand that it’s intended to mean instance data for a set of modules but, if there’s a 1:1 relationship between file and set, then just pick one term for the draft.

I don’t understand what P7 means.  Add more text to draft.

Section 3 says this about Content data: “It MAY include entity-tags and timestamps as defined in [RFC8040]”.  How is this possible?  RFC 8040 only returns such data in HTTP headers; there’s no defined encoding for putting the data into instance data.

Section 3 also says “It MAY include an explicit tag for default values as defined in
[RFC6243] and [RFC8040]”.   Do you mean, the “default” attribute defined in Section 6 in RFC 6243 and Section 4.8.9 in RFC 8040?  The text should be more explicit.

Section 3 also contains paragraphs beginning with: “It MAY include implementation specific metadata.”  and “It MAY include implementation specific XML attributes.”  I think these two paragraphs should be merged and a sentence added noting how metadata is encoded for JSON and XML.

s/A single instance data set/An instance data set/  (since already there is only one)

Section 3 says: “Instance data files MAY contain partial data sets. This means mandatory, min-elements or require-instance=true constrains MAY be violated.”  Why?  This means validations may fail.

Generally, I feel that the part of Section 3 describing the content should be replaced with a statement that any valid response for <get-data> or GET on a top-level resource is okay.  If this is not the case, then the draft should still start with this statement and them list out any exceptions.

“Metadata” is a general term, please use another term in Section 3, or spell out what is intended.  BTW, there isn’t a node in the YANG module called “metadata”, leading the extra confusion.

Where is the tree diagram?  All drafts defining YANG modules should include a YANG tree diagram for each module. 

Actually, reverting on my comment above, I recommend replacing Section 3 with the familiar 3-tuple: tree-diagram, example, YANG module.  In particular, I would delete all the text that can be described (and better at that) by the YANG module.  This eliminates duplication of content, which is both less to read and eliminates possibly conflicting information.

Stopping my review before section 3.1.

Kent // contributor

---

From: Balázs Lengyel <[email-masked]>
Date: Mon, 25 Mar 2019 21:21:06 +0000

Hello Kent,

Sadly you are right. So I should probably remove it.

On the other hand IMHO it is could be useful info. What would you
think, if I found an alternative encoding for it. (It would be
optional.)

It could be encoded as metadata on the content-data
container/anydata. It could give a quick indication whether the
data has changed. If config data is read/provided periodically, it
could be useful.

So is it useful enough to bother? Is the encoding as metadata OK?

Or should I just remove it?

regards Balazs

On 2019. 03. 25. 8:03, Kent Watsen
wrote:

Section 3 says
this about Content data: “It MAY include entity-tags and
timestamps as defined in [RFC8040]”.
How is this possible?  RFC 8040 only returns such data in HTTP
headers; there’s no defined encoding for putting the data into
instance data.

--
Balazs Lengyel Ericsson Hungary Ltd.
Senior Specialist
Mobile: +36-70-330-7909 email: [email-masked]

---

From: Balázs Lengyel <[email-masked]>
Date: Mon, 25 Mar 2019 21:52:07 +0000

Thanks for the comments, see below. Balazs

The Abstract says “re-using the same format as the reply to a
<get> operation/request”.  At first, I was going to
suggest replacing <get> with <get-data>, but I
question if this is correct since the draft supports JSON, which
neither NETCONF RPC is able to return.

BALAZS: Restconf does have a get method. I
thought I could refer to it as a get request.

The Terminology section is missing an entry for “instance
data set”.

BALAZS:
https://tools.ietf.org/html/draft-ietf-netmod-yang-instance-file-format-02#section-1
second definition is instance data set

BALAZS:
https://tools.ietf.org/html/draft-ietf-netmod-yang-instance-file-format-02#section-3
paragraph starting with "Metadata, information about the data set
itself SHALL be included ..." explains it. Shall I add a
cross-reference?

Re: P4, if file == 1 “set”, then the distinction becomes
unclear (see earlier comment about missing “set” term).  I
understand that it’s intended to mean instance data for a set of
modules but, if there’s a 1:1 relationship between file and set,
then just pick one term for the draft.

BALAZS: The relationship is 1-to-1. You don't
know how often I got the question: can I put multiple YANG modules
into a file?

Others commented, that instance data sets may be used without a
file e.g. transfered on the wire e.g. as part of some IPC
mechanism, so separating the set from the file seems like a good
idea.

I
don’t understand what P7 means.  Add more text to draft.

BALAZS:  Explained in
https://tools.ietf.org/html/draft-ietf-netmod-yang-instance-file-format-02#section-3

Instance data files MAY contain partial data sets. This means
mandatory, min-elements or require-instance=true constrains MAY be
violated.
So if you have a module with 2 mandatory subtrees, you are free to specify data only for one of them.

Section
3 says this about Content data: “It MAY include entity-tags
and timestamps as defined in [RFC8040]”.
How is this possible?  RFC 8040 only returns such data in
HTTP headers; there’s no defined encoding for putting the data
into instance data.

BALAZS: See separate mail.

BALAZS: yes,
and I will update the text.

BALAZS: OK

BALAZS: OK

Section 3 says: “Instance data files MAY contain partial data sets. This means mandatory, min-elements or require-instance=true constrains MAY be
violated.”  Why?  This means validations may fail.

BALAZS:

E.g. If you want to document the module set a server supports, you
may omit the namespace. THe name/revision clearly identifies each
YAM. While the namespace is mandatory in the module it is not
needed to document the module set.

E.g. if you want interface statistics as
diagnostics data, it may be enough to document the counters, but
not the if-index.

E.g. Configuration to be preloaded may be
provided partially by a file, and partially by the system
itself, or in more additional files.

BALAZS: In the previous version I used that
kind of definition. However Jurgen and Rob W. objected strongly
against defining the returned data using the
get-data/get.

We might also have partial-data set, we might have a mix of config
and state data, which might not be valid get-data replies.

BALAZS:
Sorry, I am missing something. Where do you see the metadata node
in the module? Or which module do you mean?

Metadata means data about the data. IMHO this is exactly the case
for the data listed in section 3. It is data about the
content-data.

What other term would you prefer?

BALAZS:
Sorry, I will add it.

BALAZS: I
will reduce duplication, however IMHO

some
overview text is useful,

text
about file-name, single-set in a file, json/xml encoding etc.
should not be in the module

the
target pointer needs some explanation, better kept in the
text, then in the module

Kent // contributor

_______________________________________________
netmod mailing list
[email-masked]
https://www.ietf.org/mailman/listinfo/netmod