karimra / gnmic

gNMIc is a gNMI CLI client and collector
https://gnmic.kmrd.dev
Apache License 2.0
217 stars 32 forks source link

update-path/update-value doesn't take into account Encoding value json_ietf #118

Closed pklepikov closed 3 years ago

pklepikov commented 4 years ago

I try to gnmic client to update some values on SRLinux platform, which supports json_ietf encoding.

Here is a simple example with hostname change:

gnmic --config gnmiClient_srl.yaml set --update-path /system/name/host-name --update-value new_hostname

Debug message:

sending gNMI SetRequest: prefix='<nil>', delete='[]', replace='[]', update='[path:{elem:{name:"system"} elem:{name:"name"} elem:{name:"host-name"}} val:{json_val:"\"new_hostname\""}]', extension='[]' to 192.168.1.1:57400

error sending set request: failed sending SetRequest to '192.168.1.1:57400': rpc error: code = InvalidArgument desc = Field type (0) is not supported

If i try to change hostname using another method, everything is successfully changed Method-1:

gnmic --config gnmiClient_srl.yaml set --update /system/name/host-name:::json_ietf:::IXR-6-4

Method-2

gnmic --config gnmiClient_srl.yaml set --update-path / --update-file change_name.json

Complete log for all combinations log.txt

karimra commented 4 years ago

Hi @pklepikov

Yes, it's a little ugly but that's how it works right now. Using --update-path and --update-value defaults to using value type json, this was to facilitate interacting with SROS. if you want a specific type (one of "json", "json_ietf", "string", "int", "uint", "bool", "decimal", "float", "bytes", "ascii") using --update or --update-file is the way to go. --update-file will take into account --encoding since files will most probably be either json or json_ietf Those are method 1 and method 2 you used.

There is an option to improve this which I'm not sure will be any better or any prettier:

Add a --update-type flag to Set cmd which takes one of "json", "json_ietf", "string", "int", "uint", "bool", "decimal", "float", "bytes", "ascii". The resulting command will look like: gnmic set --update-path /system/name/host-name --type json_ietf --update-value new_hostname

This also means a --replace-type flag needs to be added to Set cmd. Multiple --update-type and --replace-type can be used and will be matched with --update-path and --replace-path according to their order.

Resulting command:

gnmic set --update-path  /dummy/path/1 --update-type  json_ietf --update-value val1 \
          --update-path  /dummy/path/2 --update-type  string    --update-value val2 \
          --replace-path /dummy/path/3 --replace-type bool      --replace-value val3

Let me know what you think about it, or if you have any alternate way to handle these cases.

karimra commented 4 years ago

Hi @pklepikov ,

Another approach is embedding the type in the --update-value flag like this: gnmic set --update-path /dummy/path/1 --update-value json_ietf:::val1

If the value contains/starts with a ::: the --delimiter flag can be used : gnmic set --update-path /dummy/path/1 --update-value json_ietf@val1 --delimiter @

Thoughts ?