Francesco Romani has posted comments on this change. Change subject: virt: common handling of exceptions ......................................................................
Patch Set 13: (3 comments) https://gerrit.ovirt.org/#/c/54664/13/lib/vdsm/virt/api.py File lib/vdsm/virt/api.py: Line 31: Line 32: def method(func): Line 33: """ Line 34: Decorate an instance method, and return a response according to the Line 35: outcome of the call. > Can you separate paragraphs with empty line? Done Line 36: If the method returns None, return a plain success response. Line 37: If the method wants to augment the success response, it could return Line 38: a dict. The dict items will be added to the success response. Line 39: The method could override the success response message this way. https://gerrit.ovirt.org/#/c/54664/13/tests/virt_api_test.py File tests/virt_api_test.py: Line 46: vmId = 'bar' Line 47: res = self.vm.succeed_with_args('vmName', vmName, 'vmId', vmId) Line 48: self.assertEquals(response.is_error(res), False) Line 49: self.assertEquals(res['args'][0], 'vmName') Line 50: self.assertEquals(res['args'][1], vmName) > We can simplify this to: Done Line 51: Line 52: def test_success_with_kwargs(self): Line 53: port = 0 Line 54: res = self.vm.succeed_with_kwargs(migrationPort=port) Line 52: def test_success_with_kwargs(self): Line 53: port = 0 Line 54: res = self.vm.succeed_with_kwargs(migrationPort=port) Line 55: self.assertEquals(response.is_error(res), False) Line 56: self.assertEquals(res['kwargs']['migrationPort'], port) > We can simplify this to: Done Line 57: Line 58: def test_success_with_wrong_return(self): Line 59: vmList = ['foobar'] # wrong type as per @api.method contract Line 60: self.assertRaises(TypeError, -- To view, visit https://gerrit.ovirt.org/54664 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ic186dc8fa062d8b3789c6057bba68fbbc23f311b Gerrit-PatchSet: 13 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Francesco Romani <[email protected]> Gerrit-Reviewer: Adam Litke <[email protected]> Gerrit-Reviewer: Amit Aviram <[email protected]> Gerrit-Reviewer: Dan Kenigsberg <[email protected]> Gerrit-Reviewer: Francesco Romani <[email protected]> Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Piotr Kliczewski <[email protected]> Gerrit-Reviewer: Vinzenz Feenstra <[email protected]> Gerrit-Reviewer: gerrit-hooks <[email protected]> Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
