Francesco Romani has posted comments on this change.

Change subject: v2v: tests: rename vmid to UUID to match Libvirts domain 
property
......................................................................


Patch Set 1:

(3 comments)

not a big fan of this change. Was this required in a former review, maybe from 
a different patch?

https://gerrit.ovirt.org/#/c/51616/1/tests/v2vTests.py
File tests/v2vTests.py:

Line 40: 
Line 41: import vmfakelib as fake
Line 42: 
Line 43: 
Line 44: VmSpec = namedtuple('VmSpec', ['name', 'uuid'])
this is nicer but clashes with python's uuid module.
It is not theoretical...
Line 45: 
Line 46: FAKE_VIRT_V2V = CommandPath('fake-virt-v2v',
Line 47:                             os.path.abspath('fake-virt-v2v'))
Line 48: 


Line 48: 
Line 49: 
Line 50: def _mac_from_uuid(uuid):
Line 51:     return "52:54:%s:%s:%s:%s" % (
Line 52:         uuid[:2], uuid[2:4], uuid[4:6], uuid[6:8])
...here, for example, this shadows the 'uuid' module
Could or could not be a problem, and could need fixing patches in the future.
Line 53: 
Line 54: 
Line 55: class MockVirDomain(object):
Line 56: 


Line 57:     def __init__(self, name="RHEL",
Line 58:                  uuid="564d7cb4-8e3d-06ec-ce82-7b2b13c6a611"):
Line 59:         self._name = name
Line 60:         self._uuid = uuid
Line 61:         self._mac_address = _mac_from_uuid(uuid)
here no big deal
Line 62: 
Line 63:     def name(self):
Line 64:         return self._name
Line 65: 


-- 
To view, visit https://gerrit.ovirt.org/51616
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I41abb2b7d7f5950e65710379a0655399014afd89
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Shahar Havivi <[email protected]>
Gerrit-Reviewer: Francesco Romani <[email protected]>
Gerrit-Reviewer: Jenkins CI
Gerrit-Reviewer: Nir Soffer <[email protected]>
Gerrit-Reviewer: Shahar Havivi <[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

Reply via email to