Nir Soffer has posted comments on this change. Change subject: External hypervisor VMs integration ......................................................................
Patch Set 28: Code-Review-1 (1 comment) http://gerrit.ovirt.org/#/c/33309/28/vdsm/v2v.py File vdsm/v2v.py: Line 64: return size * 1024 Line 65: elif lunit in ('tib', 't'): Line 66: return size * 1024 * 1024 Line 67: else: Line 68: raise InvalidVMConfiguration("Invalid memory unit: %r", unit) This will create this message that does no make any sense: Invalid Invalid memory unit: %r value: 'unit value' Seems that the InvalidConfiguration class you created does not fit all the uses you need. You want to raise it either with 2 values (for field_name, bad_value), or for general errors like invalid memory unit. So the simple solution is to remove the implementation of InvalidConfigration, and just call it with the message you need based on context. If you think that having a special class for 2 errors is worth the effort, create a subclass (e.g. InvalidFieldValue) that log field_name, bad_value pair. Line 69: Line 70: Line 71: def _addGeneralInfo(root, params): Line 72: e = root.find('./uuid') -- To view, visit http://gerrit.ovirt.org/33309 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I7dcfb860626a844d1d08590274b508519a33f4a3 Gerrit-PatchSet: 28 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Shahar Havivi <shav...@redhat.com> Gerrit-Reviewer: Antoni Segura Puimedon <asegu...@redhat.com> Gerrit-Reviewer: Dan Kenigsberg <dan...@redhat.com> Gerrit-Reviewer: Francesco Romani <from...@redhat.com> Gerrit-Reviewer: Nir Soffer <nsof...@redhat.com> Gerrit-Reviewer: Piotr Kliczewski <piotr.kliczew...@gmail.com> Gerrit-Reviewer: Saggi Mizrahi <smizr...@redhat.com> Gerrit-Reviewer: Shahar Havivi <shav...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
