akarneliuk/pygnmi

path format and root format

slieberth opened this issue · 5 comments

Hi Anton,

I verified the pygnmi path syntax and I am not sure, whether the pygnmi path generator implementation matches the gnmi spec ...

in client.py docstring the path is specified_
path = ['yang-module:container/container[key=value]', 'yang-module:container/container[key=value]', ..]

in https://github.com/openconfig/reference/blob/master/rpc/gnmi/gnmi-path-conventions.md I see following spec: ... the root path / is encoded as a zero length array (slice) of PathElem messages. Example declarations in several languages: Python: path = [] Note this is not the same as a path consisting of a single empty string element. A human-readable path can be formed by concatenating elements of the prefix and path using a / separator, and preceded by a leading / character. ....

in this respect I think we should verify the pygmni implementation:

  • a single root path / should be allowed. The current pygnmi implementation incorrectly converts "/" to "path { elem {} elem {} }", should be changed to "path []"
  • leading / character must be allowed in path. This is not the case with current implementation. E.g. the current pygnmi implementation incorrectly converts "/interfaces/interface" to "path { elem {} elem { name: "interfaces"} elem { name: "interface } }", should be changed to "path { elem { name: "interfaces"} elem { name: "interface } }""
  • both two options must allow origin specification: e.g. "openconfig:/" or "openconfig:interfaces:/interfaces/interface"

@anton, I am happy to change the path generator model accordingly, test against Juniper and Arista and send a pull request ... let me know ...

best regards Stefan

Hello @slieberth ,

Actually, that was a thing I added to the new release published yesterday (modification of the path processing to allow '/' and possibility to omit or add origin). Could you please check that out?

Best,
Anton

Hi Anton,

thanks for the changes, looks almost perfect, if origin and leading slash is present (e.g. openconfig-interfaces:/interfaces/interface), then origin is not detected correctly, instead the leading origin gets a path element ...

/ --> path {} --> OK!

/interfaces/interface --> path { elem {name: "interfaces"} elem {name: "interface" } --> OK!

openconfig-interfaces:interfaces/interface --> path { origin: "openconfig-interfaces" elem { name: "interfaces" } elem { name: "interface" } } --> OK!

openconfig-interfaces:/interfaces/interface --> path { elem { name: "openconfig-interfaces:" } elem { name: "interfaces" } elem { name: "interface" } --> NOK!

regards Stefan

Hey @slieberth ,

I think it is fixed now. Could you please check?
Could you please also point to the part in documentation with that part : openconfig:interfaces:/interfaces/interface. I tried to search, but not found, so built on your advise.

Best,
Anton

Hi Anton,
thanks for the change, looks good, can confirm the fix, thanks.

in respect to documentation: the doc-string in client.py:148 needs minor change:
path = ['yang-module:container/container[key=value]', 'yang-module:container/container[key=value]', ..]

maybe you should specify also a path without origin prefix. In my tests against arista, origin is completly ignored, so also xxx:/interfaces/interface does work. When testing against Cisco and Juniper I use only the path, without origin.

Greetings from Berlin
Stefan

Hello @slieberth ,

Thanks a lot. We'll add the mentioned enhancement in the next release in the documentation. With this, we will close the issue.

Best,
Anton