Saggi Mizrahi has posted comments on this change.
Change subject: jsonrpc: Implement the JsonRPC server for the next-gen API
......................................................................
Patch Set 2: (6 inline comments)
Sorry, if it wasn't for the exception raising bug I would have given you a +1
with optional comments.
....................................................
File vdsm_api/BindingJsonRpc.py
Line 40:
Line 41: def start(self):
Line 42: def threaded_start():
Line 43: self.server.serve_forever()
Line 44: t = threading.Thread(target=threaded_start,
target=self.server.serve_forever
Line 45: name='JsonRpc')
Line 46: t.setDaemon(True)
Line 47: t.start()
Line 48:
Line 64: while True:
Line 65: # self.request is the TCP socket connected to the client
Line 66: try:
Line 67: data = self.request.recv(_Size.size)
Line 68: if len(data) == 0:
!= _Size.size
Line 69: self.log.debug("Connection closed")
Line 70: return
Line 71: msgLen = _Size.unpack(data)[0]
Line 72: msg = json.loads(self.request.recv(msgLen))
Line 76: self.log.debug('Received request: %s', msg)
Line 77:
Line 78: try:
Line 79: ret = self.server.bridge.dispatch(msg['methodName'],
Line 80: msg.get('args', {}))
1 rouge space
Line 81: resp = self.buildResponse(msg['id'], ret)
Line 82: except Exception:
Line 83: self.log.warn("Dispatch error", exc_info=True)
Line 84: err = {'code': 5, 'message': 'Dispatch error'}
....................................................
File vdsm_api/Bridge.py
Line 35: try:
Line 36: fn = getattr(self, methodName)
Line 37: except AttributeError:
Line 38: error = {'code': 4,
Line 39: 'message': "Operation '%s' not supported" % name}
Method not found is -32601 according to json-rpc spec
Not that this server is in anyway compliant :)
Line 40: return {'result': result, 'error': error}
Line 41: try:
Line 42: result = fn(argobj)
Line 43: except VdsmError, e:
Line 84: if attr in self.commands:
Line 85: className, methodName = attr.split('_')
Line 86: return partial(self._dynamicMethod, className, methodName)
Line 87: else:
Line 88: raise AttributeError
You are raising the type and not an instance of the type
Line 89:
Line 90: def _getArgList(self, cmd):
Line 91: return self.commands[cmd].get('data', {}).keys()
Line 92:
Line 105: """
Line 106: The schema prefixes symbol names with '*' if they are
optional. Strip
Line 107: that annotation to get the correct symbol name.
Line 108: """
Line 109: if symName[0] == '*':
pep8
Use ''.startswith() and ''.endswith() instead of string slicing to check for
prefixes or suffixes.
startswith() and endswith() are cleaner and less error prone. For example:
Yes: if foo.startswith('bar'):
No: if foo[:3] == 'bar':
Line 110: symName = symName[1:]
Line 111: return symName
Line 112:
Line 113: # TODO: Add support for map types
--
To view, visit http://gerrit.ovirt.org/8614
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Idae0faa80ffc6a5af002a8a7151aa40dc9a6673d
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Adam Litke <[email protected]>
Gerrit-Reviewer: Adam Litke <[email protected]>
Gerrit-Reviewer: Saggi Mizrahi <[email protected]>
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
