Enhance gnmi_cli_py #4

ganglyu · 2023-08-31T05:19:28Z

Upgrade grpcio and grpcio-tools
Support origin in gnmi prefix
Print grpc error code
Support PROTO encoding for gnmi get
Ignore / in []
Support proto_bytes for gnmi set
Support multiple path and value for gnmi set, get and subscribe

Upstream PR for google/gnxi:
google#350
google#351

qiluo-msft · 2023-09-04T22:29:53Z

    print("Client receives an exception '{}' indicating gNMI server is shut down and Exiting ..."

Original error message seems useful. If you do not check err.code, will you print similar fancy message? If not, let's use if-else to print UNAVAILABLE message and general message. #Closed

Refers to: gnmi_cli_py/py_gnmicli.py:557 in 31ba981. [](commit_id = 31ba981, deletion_comment = True)

qiluo-msft · 2023-09-04T22:35:47Z

gnmi_cli_py/py_gnmicli.py

  parser.add_argument('--update_count', default=0, type=int, help='Max number of streaming updates to receive. 0 means no limit.')
  parser.add_argument('--subscribe_mode', default=0, type=int, help='[0=STREAM, 1=ONCE, 2=POLL]')
-  parser.add_argument('--encoding', default=0, type=int, help='[0=JSON, 1=BYTES, 2=PROTO, 3=ASCII, 4=JSON_IETF]')
+  parser.add_argument('--encoding', default=4, type=int, help='[0=JSON, 1=BYTES, 2=PROTO, 3=ASCII, 4=JSON_IETF]')


4

This is a valid bugfix on its own. Suggest separate into a standalone commit, not to mix with proto feature. #Closed

This change is not necessary, so I have reverted it.

qiluo-msft · 2023-09-04T22:41:39Z

gnmi_cli_py/py_gnmicli.py

+          i = 0
+          for notification in response.notification:
+            for update in notification.update:
+              with open(proto_list[i], 'wb') as fp:


[i]

Possible to index out of range? #Closed

qiluo-msft · 2023-09-04T22:43:16Z

gnmi_cli_py/py_gnmicli.py

                      ' with "@"; eg "@interfaces.json".',
                      required=False)
+  parser.add_argument('--proto', type=str, help='Output files for proto bytes',
+                      nargs="+", required=False)


"+"

You assume at least one file. If the query is expected to fail, do you allow gnmicli to have 0 args? #Closed

updated to use *

qiluo-msft · 2023-09-04T22:53:46Z

def _path_names(xpath):

Could you check the go implementation https://github.com/google/gnxi/blob/2cca83d741c326d88cc3652cfbac26161bbb7209/utils/xpath/xpath.go#L255.
Is it more robust? #Closed

Refers to: gnmi_cli_py/py_gnmicli.py:173 in 88adc85. [](commit_id = 88adc85, deletion_comment = False)

qiluo-msft · 2023-09-04T23:01:26Z

gnmi_cli_py/py_gnmicli.py

+    try:
+      proto_bytes = six.moves.builtins.open(json_value.strip('$'), 'rb').read()
+    except (IOError, ValueError) as e:
+      raise Exception('Error while loading proto: %s' % str(e))


Exception

Suggest to use ValueError. Does the error message contains the filename? If not, suggest add it to help troubleshoot. #Closed

qiluo-msft · 2023-09-04T23:07:17Z

gnmi_cli_py/py_gnmicli.py

+      raise Exception('Error while loading proto: %s' % str(e))
+    val.proto_bytes = proto_bytes
+    return val
+  elif '#' in json_value:


'#' in json_value

Is it a new feature in command line? I do not see any usage text or code comment, so hard to read. #Closed

I have added comment.

"Use '#' to indicate empty value" is kind of hacky to me.
could you just use empty python string to "to indicate empty value"?

qiluo-msft · 2023-09-04T23:08:57Z

gnmi_cli_py/py_gnmicli.py

                      '\nCan be Leaf value or JSON file. If JSON file, prepend'
                      ' with "@"; eg "@interfaces.json".',
                      required=False)
+  parser.add_argument('--value_list', type=str, help='Value for SetRequest.'


value_list

Possible to reuse original -val with nargs="+" ? #Closed

qiluo-msft · 2023-09-04T23:09:13Z

gnmi_cli_py/py_gnmicli.py

  parser.add_argument('-x', '--xpath', type=str, help='The gNMI path utilized'
-                      'in the GetRequest or Subscirbe', required=True)
+                      'in the GetRequest or Subscirbe')
+  parser.add_argument('--xpath_list', type=str, help='The gNMI path utilized'


--xpath_list

Possible to reuse original -x with nargs="+" ? #Closed

qiluo-msft · 2023-09-04T23:11:03Z

gnmi_cli_py/py_gnmicli.py

  gnmi_elems = []
+  gnmi_origin = None
+  if p_names and ":" in p_names[0]:
+    res = p_names[0].split(":", 1)


":"

If it your invention or gnmi protocol? #Closed

This is not gnmi protocol.
Google gnxi was using the same solution: google@89f51f0
Now they use prefix to support origin:
google#295
Please let me know which one is better.

According to gnmi spec: In the case that a prefix is specified, it MUST specify any required origin.
Now I use prefix to support origin

ganglyu added 7 commits August 31, 2023 15:47

Upgrade grpc package

6d8333d

Support origin in gnmi path

31ba981

Print grpc error code

c23bf8b

Support PROTO encoding for gnmi get

231c698

Ignore / in []

88adc85

Support proto_bytes for gnmi set

6d10e2a

Support multiple path and value in gnmi set, get and subscribe

4e6cd2b

ganglyu force-pushed the enhance_gnmi_cli branch from d3ecdf4 to 4e6cd2b Compare August 31, 2023 08:27

qiluo-msft reviewed Sep 4, 2023

View reviewed changes

ganglyu added 3 commits September 5, 2023 16:07

Address review comment

19b5110

Fix type

11e39a9

Fix args

0696845

ganglyu requested a review from qiluo-msft September 6, 2023 01:52

Address review comment

9f730c3

qiluo-msft approved these changes Sep 6, 2023

View reviewed changes

ganglyu closed this by deleting the head repository Sep 7, 2023

Fix indent

b95501a

ganglyu reopened this Sep 8, 2023

qiluo-msft approved these changes Sep 8, 2023

View reviewed changes

lguohan merged commit 3adf8b9 into lguohan:master Sep 8, 2023

Uh oh!

Enhance gnmi_cli_py #4

Enhance gnmi_cli_py #4

Uh oh!

Conversation

ganglyu commented Aug 31, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

qiluo-msft commented Sep 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

qiluo-msft Sep 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qiluo-msft Sep 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qiluo-msft Sep 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qiluo-msft commented Sep 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

qiluo-msft Sep 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qiluo-msft Sep 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qiluo-msft Sep 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qiluo-msft Sep 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ganglyu Sep 5, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

qiluo-msft Sep 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ganglyu Sep 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ganglyu commented Aug 31, 2023 •

edited

Loading

qiluo-msft commented Sep 4, 2023 •

edited

Loading

qiluo-msft Sep 4, 2023 •

edited

Loading

qiluo-msft Sep 4, 2023 •

edited

Loading

qiluo-msft Sep 4, 2023 •

edited

Loading

qiluo-msft commented Sep 4, 2023 •

edited

Loading

qiluo-msft Sep 4, 2023 •

edited

Loading

qiluo-msft Sep 4, 2023 •

edited

Loading

qiluo-msft Sep 4, 2023 •

edited

Loading

qiluo-msft Sep 4, 2023 •

edited

Loading

ganglyu Sep 5, 2023 •

edited

Loading

qiluo-msft Sep 4, 2023 •

edited

Loading

ganglyu Sep 6, 2023 •

edited

Loading