Nir Soffer has posted comments on this change. Change subject: xmlrpc: marshalling of Long type ......................................................................
Patch Set 3: Code-Review-1 (1 comment) http://gerrit.ovirt.org/#/c/34856/3/vdsm/rpc/BindingXMLRPC.py File vdsm/rpc/BindingXMLRPC.py: Line 49: class BindingXMLRPC(object): Line 50: def __init__(self, cif, log, port): Line 51: xmlrpclib.Marshaller.dispatch[types.LongType] = ( Line 52: lambda _, v, w: w(_STRING_VALUE % v) Line 53: ) This seems to use the value of the number being marshaled, so the result will be different depending on the value. We need a conversion which is based on the schema, not on the value. Assuming that we use this new infrastructure and remove the explicit str() in the verbs responses, we will break compatibility with older engine. For example, a verb returning free space on a device - when the device is empty, it may return value > 2**3, which will be marshaled as string, and when the disk is fuller, the value may be < 2**32, which will be marshaled as int. On the other side, engine expected to get always a string, and may fail to read the unexpected int. Look at the schema - in any place the schema promise a string for numeric value, we should always send a string, regardless of the value. Unless older engine code can handle both string and int for numeric values. Line 54: self.cif = cif Line 55: self.log = log Line 56: self.serverPort = port Line 57: -- To view, visit http://gerrit.ovirt.org/34856 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I56341566e4add6a093ba89eb81e09ee9e7a631b1 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Piotr Kliczewski <[email protected]> Gerrit-Reviewer: Antoni Segura Puimedon <[email protected]> Gerrit-Reviewer: Martin Sivák <[email protected]> Gerrit-Reviewer: Nir Soffer <[email protected]> Gerrit-Reviewer: Piotr Kliczewski <[email protected]> Gerrit-Reviewer: Saggi Mizrahi <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
