openconfig / gnmi

gRPC Network Management Interface
Apache License 2.0
464 stars 196 forks source link

gnmi_cli returns an error, when delimiter is contained in key value #6

Closed wisotzky closed 7 years ago

wisotzky commented 7 years ago

Example:

# go run gnmi_cli.go -a pe1:57400 -with_user_pass -qt s -dt p -insecure -q state/port[port-id=1/1/1]/ethernet/statistics/out-utilization
username: grpc
password: ********
E0913 20:32:05.785297   20625 gnmi_cli.go:180] cli.QueryDisplay:
        sendQueryAndDisplay(ctx, {Addrs:[pe1:57400] Target: Replica:0 Discard:false Queries:[[state port[port-id=1 1 1] ethernet statistics out-utilization]] Type:stream Timeout:30s NotificationHandler:<nil> ProtoHandler:<nil> Credentials:0xc420150440 TLS:0xb75d60 Extra:map[]}, &{PollingInterval:30s StreamingDuration:0s Count:0 countExhausted:false Delimiter:/ Display:0x7c56f0 DisplayPrefix: DisplayIndent:   DisplayType:p DisplayPeer:false Timestamp: DisplaySize:false Latency:false ClientTypes:[]}):
        client had error while displaying results:
        rpc error: code = Unimplemented desc =

While this one is working nicely:

# go run gnmi_cli.go -a pe1:57400 -with_user_pass -qt s -dt p -insecure -q state_port[port-id=1/1/1]_ethernet_statistics_out-utilization -d _
username: grpc
password: ********

update: <
  timestamp: 1505327998773031795
  prefix: <
    element: "state"
    element: "port[port-id=1/1/1]"
    element: "ethernet"
    element: "statistics"
  >
  update: <
    path: <
      element: "out-utilization"
    >
    val: <
      json_val: "0"
    >
  >
>

sync_response: true

It would be desired, if the any slash characters "/" within square brackets "[...]" are not used as delimiter.

awly commented 7 years ago

This is precisely the purpose of the -d flag.

Extra work to parse the query with any kind of escaping would add complexity that doesn't seem justified.

wisotzky commented 7 years ago

It does not add significant extra complexity.

Cross-check with my gNMI subscription tool in python https://github.com/nokia/pygnmi which support escaping. The coding effort is similar to having the delimiter configurable.

Please keep in mind, that if '/' is not working, people typically start to play around "," (comma) or ";" (semicolon). The issue is, that comma is already used by gnmi_cli to allow multiple queries by the same call. And semicolon is interpret by UNIX shell (e.g. bash). Same also would be true about backslash. Before playing around too much with separators that work, it is really recommended just to ignore separators within square brackets.

wisotzky commented 7 years ago

Maybe worth adding... In my python tool I am using the following code:

    if path:
        if path[0]=='/':
            if path[-1]=='/':
                return re.split('''/(?=(?:[^\[\]]|\[[^\[\]]+\])*$)''', path)[1:-1]
            else:
                return re.split('''/(?=(?:[^\[\]]|\[[^\[\]]+\])*$)''', path)[1:]
        else:
            if path[-1]=='/':
                return re.split('''/(?=(?:[^\[\]]|\[[^\[\]]+\])*$)''', path)[:-1]
            else:
                return re.split('''/(?=(?:[^\[\]]|\[[^\[\]]+\])*$)''', path)
    return []

While this looks complex, it allows people to have a bit of flexibility. It basically allows people to use a slash at the beginning of the path expression - which some people like to do for compatibility of xpath expressions. Also it ignores a slash at the end, as I am using the same method for the prefix - and some people using a slash at the end of prefixes. Finally I am returning the empty list [] in case of a zero-length string, as running queries against the root element in gNMI has changed to [] but was [""] before.

gcsl commented 7 years ago

fixed by 3d0217d986e4c6966729765683a38a6073dc1028

wisotzky commented 7 years ago

Looks good now. Thx