Subject: [netmod] comments on draft-yang-json-rpc-00

From: Giles Heron <[email-masked]>
Date: Tue, 4 Jul 2017 17:55:31 +0100

Hi Authors,

some quick comments on your draft.

meta-comments:

1) shouldn’t this be in the NETMOD WG and be named “draft-netmod-yang-json-rpc-00” once adopted, and “draft-issn-yang-json-rpc-00” (or whatever) for now?   Posting these comments to NETMOD at any rate :)

2) why does each page have a number 0 at the top?

3) might be worth adding a security section and some IANA considerations (even if there are no actions required from IANA)

Section 1:

4) s/ZMQ/0MQ (ref can still be [ZEROMQ])

5) s/RabbitMQ/AMQP (ref can still be [RABBITMQ])

6) you say JSON RPC 2.0 has a number of well known shortcomings.  Aren’t there really just the two - no means of describing/documenting an API method and no means to interpret data and to generate structures to receive the data?   or is that three? ;)  I almost wonder if bullets are the answer here.

7) s/lead/led in “this has lead them”

8) I think it should be “and/or use” not “and/or the use of”

9) isn’t “not portable across languages” basically the same as “break the core assumption of JSON being a language agnostic interchange format”?

10) is it really a core assumption that JSON is language agnostic?  Given the JS in its name i mean…

11) you reference the "JSON Encoding of Data Modeled with YANG” draft.  That’s RFC7951 :)

Section 2:

12) maybe it’s time to move to YANG 1.1 (RFC 7950)

13) do you need to use square brackets each time you refer to a doc? I’d just do it the first time and use the name thereafter.

Section 3:

14) s/yang/YANG

15) where you have links to sections of RFCs I’d keep the link to the section separate from any link to the whole RFC.

16) where referring to an RFC outside of square brackets I think the correct style is to have a space between “RFC” and the RFC number.

17) might be worth explaining why use of must, when, leafref and identityref can harm interoperability.

18) also might be worth enumerating the characters to be discouraged.

19) it’s probably worth breaking the input and output for the RPC out as separate sub-sections rather than having the input in the main part of section 3 and the output as one sub-section of section 3.4

20) it’s probably worth explaining that figure 3 is the positional form and figure 4 is named.   You do that later on so not to do it for your first example is somewhat confusing.

21) it’s probably also worth explaining that the mappings for the positional form are a difference wrt RFC7951 (since YANG-modelled data is always carried in named form when mapped into XML or JSON).

Section 3.2:

22) link missing under 7.13.3

Section 3.3:

23) from my understanding of YANG extensions you need to write a YANG model for idl and publish that.

24) seems you’ve actually defined two extensions here - idl:value-type and idl:implemented-by

Section 3.4:

25) the output rules seem to conflict with the input rules re lists (one reason why I think separating those out into separate subsections makes sense)

Section 3.4.1:

26) I’m not sure having {“key” : “value”} is a good example for anyxml in that it tends to imply you have a model somewhere and know the keys?  The same occurs in Fig 23 (sec 3.4.3)

Section 3.4.3:

27) in fig 26 I think the result should be “object”, not “test-object”?

Section 3.6:

28) I think this is section 6.11 not 6.1 of RFC7951?

29) worth explaining why the representation doesn’t match JSON semantics

30) is this YANG path stuff equivalent to the actions in YANG 1.1?

Section 3.6.1:

31) please define QName

Section 3.6.3:

32) please delete the word “section” after “Section 3.6.1"

Section 4:

33) the Zero MQ and JSON RPC refs seem to be back to front

34) you’ve got PJZMQ pointing at www.jabsorb.org <http://www.jabsorb.org/>

---

From: Anton Ivanov <[email-masked]>
Date: Tue, 4 Jul 2017 18:14:10 +0100

> Hi Authors,

Hi Giles, thanks for the comments. We will take most of them onboard and 
release an updated version as soon as practical.

> 1) shouldn’t this be in the NETMOD WG and be named 
> “draft-netmod-yang-json-rpc-00” once adopted, and 
> “draft-issn-yang-json-rpc-00” (or whatever) for now? Posting these 
> comments to NETMOD at any rate :)

Fair point.

> 2) why does each page have a number 0 at the top?

No idea, will look at the xml.

>
> 3) might be worth adding a security section and some IANA 
> considerations (even if there are no actions required from IANA)

Ok.

> 4) s/ZMQ/0MQ (ref can still be [ZEROMQ])

Ok. Will address in next revision.

> 5) s/RabbitMQ/AMQP (ref can still be [RABBITMQ])

OK. Will address in next revision.

>
> 6) you say JSON RPC 2.0 has a number of well known shortcomings. 
>  Aren’t there really just the two - no means of describing/documenting 
> an API method and no means to interpret data and to generate 
> structures to receive the data?   or is that three? ;)  I almost 
> wonder if bullets are the answer here.

There are a couple of more which are outside the scope of the draft for 
the duration - lack of async support and callback calling conventions.

> 7) s/lead/led in “this has lead them”

Ok. Will fix.

> 8) I think it should be “and/or use” not “and/or the use of”

Ditto.

>
> 9) isn’t “not portable across languages” basically the same as “break 
> the core assumption of JSON being a language agnostic interchange format”?

It is.

>  Given the JS in its name i mean…

While it was not originally, it quickly became one and is presently 
assumed to be language agnostic by most developers.

> 11) you reference the "JSON Encoding of Data Modeled with YANG” draft. 

Guilty as charged, will be fixed.

> 12) maybe it’s time to move to YANG 1.1 (RFC 7950)

We need to double-check everything for any of the changes, the intention 
was to do this in the next revision.

>
> 13) do you need to use square brackets each time you refer to a doc? 
> I’d just do it the first time and use the name thereafter.

LyX does it for me and the rest is a f(laziness).

> 14) s/yang/YANG

Guilty as charged, will be fixed. I thought I caught all of these.

>
> 15) where you have links to sections of RFCs I’d keep the link to the 
> section separate from any link to the whole RFC.

Thanks, will take that onboard.

>
> 16) where referring to an RFC outside of square brackets I think the 
> correct style is to have a space between “RFC” and the RFC number.

Thanks, will take that onboard.

> identityref can harm interoperability.

Fair point. We will expand this in the next revision.

> 18) also might be worth enumerating the characters to be discouraged.

Ditto.

>
> 19) it’s probably worth breaking the input and output for the RPC out 
> as separate sub-sections rather than having the input in the main part 
> of section 3 and the output as one sub-section of section 3.4

It is an artefact of writing the spec after writing the PoC code we did 
initially. The PoC code reused a lot here so the spec followed what we 
did. We will fix that.

>
> 20) it’s probably worth explaining that figure 3 is the positional 
> form and figure 4 is named.   You do that later on so not to do it for 
> your first example is somewhat confusing.

Thanks for noting, will fix.

>
> 21) it’s probably also worth explaining that the mappings for the 
> positional form are a difference wrt RFC7951 (since YANG-modelled data 
> is always carried in named form when mapped into XML or JSON).

OK, we will look at that.

> 24) seems you’ve actually defined two extensions here - idl:value-type 

I will refer this portion to David Spence as he is the author of this 
part to be addressed in the next revision.

>
>
> Section 3.4:
>
> 25) the output rules seem to conflict with the input rules re lists 
> (one reason why I think separating those out into separate subsections 
> makes sense)

>
>
> Section 3.4.1:
>
> 26) I’m not sure having {“key” : “value”} is a good example for anyxml 
> in that it tends to imply you have a model somewhere and know the 
> keys?  The same occurs in Fig 23 (sec 3.4.3)

One of the intended applications of anyxml is exactly that: "I have a 
model, but I do not want to give the library which manages the actual 
RPC implementation access to it" - transporting data opaquely across a 
modelled connection. We will review the text to clarify this properly.

We will also look at the remaining comments/noted errors and fix them.

> 34) you’ve got PJZMQ pointing at www.jabsorb.org <http://www.jabsorb.org>
>

Once again, thank you very much for reviewing it, we will address the 
comments in the next revision.

A.