Yaniv Bronhaim has posted comments on this change. Change subject: jsonrpc: introduce new client ......................................................................
Patch Set 10: (3 comments) https://gerrit.ovirt.org/#/c/64502/10/lib/vdsm/client.py File lib/vdsm/client.py: Line 108: ServerError will be raised if the command encountered execution error: Line 109: Line 110: vdsm.client.ServerError: Vdsm request failed Line 111: (code=4, message=Virtual machine already exists) Line 112: you repeat the errors description twice (here and in the call method). I guess that the next update won't update both locations so better to have only one description Line 113: Line 114: Please note that if the client isn't a context manager, it should be Line 115: closed at the end of the run: Line 116: Line 192: ''' Line 193: if args is None: Line 194: args = {} Line 195: if timeout is None: Line 196: timeout = self.default_timeout why not just have it as default values? like "def call(self, method, args={}, timeout=self.default_timeout): " I didn't understand nir's comment about it. why a modification will effect next calls? Line 197: req = yajsonrpc.JsonRpcRequest(method, args, reqId=str(uuid.uuid4())) Line 198: try: Line 199: responses = self._client.call(req, timeout=timeout) Line 200: except EnvironmentError as e: Line 224: try: Line 225: self.close() Line 226: except Exception: Line 227: if t is None: Line 228: raise why is this type check ? -- 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: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Irit Goihman <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Irit Goihman <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Martin Sivák <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Simone Tiraboschi <[email protected]> Gerrit-Reviewer: Yaniv Bronhaim <[email protected]> Gerrit-Reviewer: gerrit-hooks <[email protected]> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list -- [email protected] To unsubscribe send an email to [email protected]
