Irit Goihman has posted comments on this change.
Change subject: wip: jsonrpc: introduce new client
......................................................................
Patch Set 9:
(10 comments)
https://gerrit.ovirt.org/#/c/64502/8/lib/vdsm/client.py
File lib/vdsm/client.py:
Line 17: #
Line 18: # Refer to the README and COPYING files for full details of the license
Line 19: #
Line 20: '''
Line 21: vdsm client
This is not a jsonrpc client (generic client for server using
jsonrpc) but
Done
Line 22:
Line 23: This is a simple client which uses jsonrpc protocol introduced on ovirt 3.5.
Line 24: This client is not aware of the available methods and parameters.
Line 25:
Line 26: The user should consult the schema to construct the wanted command.
Line 27:
Line 28: The client is invoked with:
Line 29:
Line 30: cli = client.connect(host, port, ssl)
The common way to show code examples is to indent them with 4 spaces,
the #
Done
Line 31:
Line 32: For example:
Line 33:
Line 34: cli = client.connect('localhost', 54321, True)
Line 36: Failure will result in ClientError exception.
Line 37:
Line 38: Invoking commands:
Line 39:
Line 40: cli.call('Host.getVMList')
Maybe we can make args optional to make it nicer to call methods
without ar
Done
Line 41:
Line 42: cli.call('VM.getStats', {'vmID':
'bc26bd11-ee3b-4a56-80d4-770f383a47b9'})
Line 43:
Line 44: The call method expects method name and a dictionary (optional) containing all
Line 44: The call method expects method name and a dictionary (optional) containing all
Line 45: mandatory parameters, as they appear in the schema.
Line 46:
Line 47: Complex parameters are also supported and share the same behavior.
Line 48:
We need to see an example result, and explain about getting the exact
resul
I will address this comment in the next change
Line 49: '''
Line 50:
Line 51:
Line 52: from __future__ import absolute_import
Line 47: Complex parameters are also supported and share the same behavior.
Line 48:
Line 49: '''
Line 50:
Line 51:
One empty line here is enough.
Done
Line 52: from __future__ import absolute_import
Line 53:
Line 54: import uuid
Line 55:
Line 90: self.code = code
Line 91: self.message = message
Line 92:
Line 93:
Line 94: class _Client(object):
These should come after the public apis.
Done
Line 95: def __init__(self, client):
Line 96: self._client = client
Line 97: self.default_timeout = yajsonrpc.CALL_TIMEOUT
Line 98:
Line 110:
Line 111: resp = responses[0]
Line 112: if resp.error:
Line 113: raise ServerError(resp.error['code'],
resp.error['message'])
Line 114:
Let document if we may get here multiple responses, when I see this
code I
same
Line 115: return resp.result
Line 116:
Line 117: def close(self):
Line 118: self._client.close()
Line 113: raise ServerError(resp.error['code'],
resp.error['message'])
Line 114:
Line 115: return resp.result
Line 116:
Line 117: def close(self):
Let keep empty line after raising/returning - the rest of the
function is h
Done
Line 118: self._client.close()
Line 119:
Line 120: def __enter__(self):
Line 121: return self
Line 118: self._client.close()
Line 119:
Line 120: def __enter__(self):
Line 121: return self
Line 122:
Here is a good place for the context manager interface.
Done
Line 123: def __exit__(self, t, v, tb):
Line 124: try:
Line 125: self.close()
Line 126: except Exception:
Line 120: def __enter__(self):
Line 121: return self
Line 122:
Line 123: def __exit__(self, t, v, tb):
Line 124: try:
This and the exceptions are the only public apis, so they should be
at the
Done
Line 125: self.close()
Line 126: except Exception:
Line 127: if t is None:
--
To view, visit
https://gerrit.ovirt.org/64502
To unsubscribe, visit
https://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Idd45d7e88bf2246beaf30550b12201917f32c354
Gerrit-PatchSet: 9
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Irit Goihman <igoihman(a)redhat.com>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Martin Sivák <msivak(a)redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer(a)redhat.com>
Gerrit-Reviewer: Simone Tiraboschi <stirabos(a)redhat.com>
Gerrit-Reviewer: gerrit-hooks <automation(a)ovirt.org>
Gerrit-HasComments: Yes