Change in vdsm[master]: contrib: Simple jsonrpc client
piotr.kliczewski at gmail.com
piotr.kliczewski at gmail.com
Tue Jan 19 08:03:27 UTC 2016
Piotr Kliczewski has posted comments on this change.
Change subject: contrib: Simple jsonrpc client
......................................................................
Patch Set 7:
(8 comments)
https://gerrit.ovirt.org/#/c/35181/7/contrib/jsonrpc
File contrib/jsonrpc:
Line 25: methods and parameters. The user should consult the schema to construct request
Line 26: that make sense. Future version should parse the schema and provide online
Line 27: help.
Line 28:
Line 29: Invoking simple methods:
> The :: suffix is intentional, this is rst format for a code block, please k
What is its value?
Line 30:
Line 31: # jsonrpc Host.getVMList
Line 32: ['b3f6fa00-b315-4ad4-8108-f73da817b5c5']
Line 33:
Line 38:
Line 39: For invokinng methods with many or complex parameters, you can read the
Line 40: parameters from a file:
Line 41:
Line 42: # jsonrpc VM.create -f vm-create
> We like to send json files, not some non-standard key=value format.
key-value format is how we want to pass params. Having a complete message looks like bad idea. We know the method and we want to pass args. I think that we do not need more.
Line 43: ...
Line 44:
Line 45: It is also possible to read parameters from standard input, creating complex
Line 46: parameters interactively:
Line 46: parameters interactively:
Line 47:
Line 48: # jsonrpc StorageDomain.activate -f -
Line 49: storagedomainID=75ab40e3-06b1-4a54-a825-2df7a40b93b2,
Line 50: storagepoolID=b3f6fa00-b315-4ad4-8108-f73da817b5c5
> The previous format was better. Using simple arguments we cannot express js
We can have multiline args but we should not keep a complete message in a file.
Line 51: (Press Ctrl + D to finish)
Line 52:
Line 53: """
Line 54:
Line 100: pretty_print(resp.result)
Line 101: finally:
Line 102: client.close()
Line 103: except Exception as e:
Line 104: fail("Connection failed: %s" % e)
> The jsonrpc changes are good, but the original error handling was much bett
Will improve
Line 105:
Line 106:
Line 107: def option_parser():
Line 108: parser = argparse.ArgumentParser()
Line 109: parser.add_argument('-a', '--host', dest="host", default="localhost",
Line 110: help='host address (default localhost)')
Line 111: parser.add_argument('method', help='remote method name')
Line 112: group = parser.add_mutually_exclusive_group()
Line 113: group.add_argument('-p', '--params', help='params [param=value ...]')
> We don't wan to use -p, using the params as key=value pairs on the command
There was code where we checked whether we provided file or params directly. It was written to be exclusive so I used mutual exclusion group which requires a param to be optional param. In that way the check is done by argparse instead of doing it manually.
Line 114: group.add_argument('-f', '--file', dest='file',
Line 115: help="read method parameters from json file. Set to"
Line 116: " '-' to read from standard input")
Line 117: return parser
Line 151: keys.sort()
Line 152: for element in keys:
Line 153: representation = pprint.pformat(resp[element]).replace(
Line 154: '\n', '\n\t' + ' ' * len(element + ' = ') + '\t')
Line 155: print('\t' + '\t%s = %s' % (element, representation))
> Why do we need this? pprint.pprint knows how to format dicts nicely
Done
Line 156: else:
Line 157: pprint.pprint(resp)
Line 158:
Line 159:
Line 153: representation = pprint.pformat(resp[element]).replace(
Line 154: '\n', '\n\t' + ' ' * len(element + ' = ') + '\t')
Line 155: print('\t' + '\t%s = %s' % (element, representation))
Line 156: else:
Line 157: pprint.pprint(resp)
> The previous way to pretify things was better - json format is much better
We do not get json response. It is already processed at this point.
Line 158:
Line 159:
Line 160: def fail(msg):
Line 161: sys.stderr.write("%s: %s\n" % (os.path.basename(sys.argv[0]), msg))
https://gerrit.ovirt.org/#/c/35181/7/lib/yajsonrpc/stompreactor.py
File lib/yajsonrpc/stompreactor.py:
Line 557: )
Line 558: )
Line 559:
Line 560:
Line 561: def SimpleClient(host, port=54321, ssl=True):
> Lets separate this change, looks nice addition regardless of the client.
Done
Line 562: """
Line 563: Returns JsonRpcClient able to receive jsonrpc messages and notifications.
Line 564: It is required to provide a host where we want to connect, port and whether
Line 565: we want to use ssl (True by default). Other settings use defaults and if
--
To view, visit https://gerrit.ovirt.org/35181
To unsubscribe, visit https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia6273eabf6f3601602659d1e4e748d8025ae8084
Gerrit-PatchSet: 7
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Adam Litke <alitke at redhat.com>
Gerrit-Reviewer: Allon Mureinik <amureini at redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <toni+ovirt at midokura.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Federico Simoncelli <fsimonce at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Jenkins CI RO
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczewski at gmail.com>
Gerrit-Reviewer: Saggi Mizrahi <smizrahi at redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation at ovirt.org>
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list