New gNMI #2454
New gNMI #2454
Conversation
Humblesaw
force-pushed
the
new_gnmi
branch
2 times, most recently
from
86e3267 to
8cac46e
Compare
jktjkt
left a comment
jktjkt
left a comment
There was a problem hiding this comment.
We talked about this on Slack -- this change apparently causes libyang to skip printing of namespace prefixes for non-top-level nodes, even if they appear as a root node in the resulting JSON. That's a big change which should be properly explained. A commit that says "printer json FEATURE no namespace for inner nodes" is not a proper explanation because it doesn't say why this change needs to happen -- it's simply saying that you're making this change. So, why is it needed, and why is it a correct change?
Now, RFC 7950 is silent about this topic, but given that all the JSON examples from the RESTCONF RFC always use namespace-qualified names even when printing subtrees, I think that keeping them namespace-qualified is a good thing. Also, this is about producing this JSON; have you checked that libyang can parse these JSONs, and is there a unit test for that?
Also, the PR currently contains two commits (which is a good thing), but the first one introduces a build regression because of its use of LYD_PRINT_WITHSIBLINGS. This is then fixed by the follow-up commit, which means that git bisection will not work. That's a bug.
| CHECK_LYD_STRING_PARAM(IN_MODEL, TEXT, LYD_JSON, PRINT_OPTION) | ||
|
|
||
| static void | ||
| test_baretop_leaf(void **state) |
There was a problem hiding this comment.
This test is checking whether you can parse a "content of a JSON field" when its value is given out-of-band (and properly NS-qualified).
Have you added a test which checks that you can still parse a JSON like {"al":25}?
There was a problem hiding this comment.
Yes, I added it and I also added a test for printing JSON.
There was a problem hiding this comment.
I think that this should be said explicitly. E.g., the docs for LYD_PRINT_JSON_NO_NESTED_PREFIX should say that parsing such a content will likely require some out-of-band information, and that lyd_parse_value_fragment is suitable for this purpose.
There was a problem hiding this comment.
Ok, I added more info on that.
Humblesaw
force-pushed
the
new_gnmi
branch
2 times, most recently
from
7920a64 to
9e2ba29
Compare
Ok, this is only needed for the gNMI server that I'm currently working on. I added a flag that modifies the behavior of the JSON printer so that the current functionality can remain. I also added a test for it and correctly separated the commits (thanks for the catch). |
Humblesaw
force-pushed
the
new_gnmi
branch
3 times, most recently
from
01f89c3 to
a961f37
Compare
Humblesaw
force-pushed
the
new_gnmi
branch
7 times, most recently
from
8b83af6 to
cf2a851
Compare
New functionality allowing parsing only JSON values and adding them to the nodes specified by path.
New flag LYD_PRINT_NO_JSON_NESTED_PREFIX makes it possible to only include the module name in the JSON data if it corresponds to the top-level node in the schema. This is useful if you need libyang to only print a subtree of the data which you may then wrap.
3 participants
No description provided.