Change in vdsm[master]: External hypervisor VMs integration
nsoffer at redhat.com
nsoffer at redhat.com
Mon Nov 3 00:34:27 UTC 2014
Nir Soffer has posted comments on this change.
Change subject: External hypervisor VMs integration
......................................................................
Patch Set 16:
(4 comments)
http://gerrit.ovirt.org/#/c/33309/16/vdsm/v2v.py
File vdsm/v2v.py:
Line 63: return size * 1024
Line 64: elif unit in ('TiB', 'T'):
Line 65: return size * 1024 * 1024
Line 66: else:
Line 67: raise InvalidVMConfigurationException('Invalid memory unit: ' + unit)
Better use "description: %r" % value, so we can see what was the bad value. For example, if it was empty string we will see:
Invalid memory unit: ''
Which is more helpful then:
Invalid memory unit:
Same comment apply to other exceptions bellow
Line 68:
Line 69:
Line 70: def _addGeneralInfo(root, params):
Line 71: e = root.find('./uuid')
Line 75: e = root.find('./currentMemory')
Line 76: if e is not None:
Line 77: try:
Line 78: size = int(e.text)
Line 79: except:
Handle only ValueError - any other error is not expected.
Same apply to other int() calls bellow
Line 80: raise InvalidVMConfigurationException('Invalid memory value: ' +
Line 81: e.text)
Line 82: unit = e.get('unit', 'KiB')
Line 83: params['memSize'] = _memToMb(size, unit)
Line 123: i['dev'] = target.get('dev')
Line 124: params['networks'].append(i)
Line 125:
Line 126:
Line 127: class InvalidVMConfigurationException(Exception):
No need to add "Exception" to the name. What elese can be "InvalidVMConfiguration"?
Line 128: def __init__(self, message):
Line 129: self.message = message
Line 130:
Line 131: def __str__(self):
Line 128: def __init__(self, message):
Line 129: self.message = message
Line 130:
Line 131: def __str__(self):
Line 132: return self.message
It is better not to implement __str__ since Exception builtin implementation is better - it will show the class name in a traceback or when you log the instance.
For rexample:
>>> class InvalidVMConfiguration(Exception):
... pass
...
>>> raise InvalidVMConfiguration("description")
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
v2v.InvalidVMConfiguration: description
If you don't implement __str__, there is no need to implement __init__.
--
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: 16
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi <shavivi at redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Francesco Romani <fromani at redhat.com>
Gerrit-Reviewer: Nir Soffer <nsoffer at redhat.com>
Gerrit-Reviewer: Shahar Havivi <shavivi at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list