Change in vdsm[master]: properties: Add properties module
gerrit-hooks has posted comments on this change. Change subject: properties: Add properties module .. Patch Set 10: * Update tracker: IGNORE, no Bug-Url found * Set MODIFIED::IGNORE, no Bug-Url found. -- To view, visit https://gerrit.ovirt.org/40822 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I62175172dddce7f84319e3c8669747994e06a697 Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir SofferGerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Jenkins CI RO Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: properties: Add properties module
Adam Litke has submitted this change and it was merged. Change subject: properties: Add properties module .. properties: Add properties module Properties are reusable objects similar to builtin property() function, adding input validation and initialization guarantees. Examples class Foo(properties.Owner): uuid = properties.UUID(required=True) format = properties.Enum(values=("cow", "raw"), required=True) size = properties.Integer(minval=0) name = properties.String() def __init__(self, uuid, format, size=0): self.uuid = uuid self.format = format self.size = size Creating an object with invalid properties or with uninitialized required properties would raise: Foo("not-a-uuid", "raw") Foo("49d8842d-43e8-4c33-b588-b5538df4ed8a", "invalid format") Foo("49d8842d-43e8-4c33-b588-b5538df4ed8a", "raw", size=-1) Attempting to set invalid value would raise: foo = Foo("49d8842d-43e8-4c33-b588-b5538df4ed8a", "raw") foo.size = -1 # Raises ValueError Accessing uninitialized properties does not raise AttributeError. Instead, the default value is returned: foo.name # None Change-Id: I62175172dddce7f84319e3c8669747994e06a697 Signed-off-by: Nir SofferReviewed-on: https://gerrit.ovirt.org/40822 Continuous-Integration: Jenkins CI Tested-by: Adam Litke Reviewed-by: Adam Litke --- M lib/vdsm/Makefile.am A lib/vdsm/properties.py M tests/Makefile.am A tests/properties_test.py M vdsm.spec.in 5 files changed, 748 insertions(+), 0 deletions(-) Approvals: Adam Litke: Verified; Looks good to me, approved Jenkins CI: Passed CI tests -- To view, visit https://gerrit.ovirt.org/40822 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: merged Gerrit-Change-Id: I62175172dddce7f84319e3c8669747994e06a697 Gerrit-PatchSet: 10 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer Gerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Jenkins CI RO Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: gerrit-hooks ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: properties: Add properties module
Adam Litke has posted comments on this change. Change subject: properties: Add properties module .. Patch Set 9: Code-Review+2 -- To view, visit https://gerrit.ovirt.org/40822 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I62175172dddce7f84319e3c8669747994e06a697 Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir SofferGerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Jenkins CI RO Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: properties: Add properties module
Adam Litke has posted comments on this change. Change subject: properties: Add properties module .. Patch Set 9: Verified+1 No users, so verified using supplied tests. -- To view, visit https://gerrit.ovirt.org/40822 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I62175172dddce7f84319e3c8669747994e06a697 Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir SofferGerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Jenkins CI RO Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: properties: Add properties module
Nir Soffer has posted comments on this change. Change subject: properties: Add properties module .. Patch Set 9: This version address Adam comments from version 6. -- To view, visit https://gerrit.ovirt.org/40822 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I62175172dddce7f84319e3c8669747994e06a697 Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir SofferGerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Jenkins CI RO Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: properties: Add properties module
gerrit-hooks has posted comments on this change. Change subject: properties: Add properties module .. Patch Set 9: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/40822 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I62175172dddce7f84319e3c8669747994e06a697 Gerrit-PatchSet: 9 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir SofferGerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Jenkins CI RO Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: properties: Add properties module
Nir Soffer has posted comments on this change. Change subject: properties: Add properties module .. Patch Set 6: (5 comments) https://gerrit.ovirt.org/#/c/40822/6/tests/propertiesTests.py File tests/propertiesTests.py: Line 74: self.value = value Line 75: Line 76: def test_required(self): Line 77: # Required property set to None is considered unset Line 78: self.assertRaises(ValueError, self.Cls) > Probably yes. Implemented in latest version. Line 79: Line 80: Line 81: class PropertyDefaultTests(VdsmTestCase): Line 82: Line 112: obj = self.Cls() Line 113: self.assertEqual(obj.value, "2") Line 114: Line 115: def test_allowed(self): Line 116: for value in ("1", "2", "3"): > I guess we cannot and should not support this. Added a test showing that this work in the current implementation. We should consider forbidding this later. Line 117: obj = self.Cls() Line 118: obj.value = value Line 119: self.assertEqual(obj.value, value) Line 120: Line 121: def test_forbidden(self): Line 122: obj = self.Cls() Line 123: self.assertRaises(ValueError, setattr, obj, "value", "4") Line 124: Line 125: def test_none(self): > I think I changed it to make it easier to handle missing value in a dict as We can think about using a special sentinel value (e.g. object()) instead of None. Line 126: obj = self.Cls() Line 127: obj.value = None Line 128: self.assertEqual(None, obj.value) Line 129: Line 270: self.assertEqual(obj.value, 3.14) Line 271: Line 272: def test_invalid(self): Line 273: obj = self.Cls() Line 274: self.assertRaises(ValueError, setattr, obj, "value", "not a float") > No, I check for float. I'm not big fan of type automatic type conversion, b Using integers for float means you can use a value that cannot be converted to float: >>> bigint = 2**100 >>> float(bigint) == bigint Traceback (most recent call last): File "", line 1, in OverflowError: long int too large to convert to float You should use the correct type, this library will not do any conversions. Line 275: Line 276: def test_none(self): Line 277: obj = self.Cls() Line 278: obj.value = None Line 290: self.assertEqual(1.0, obj.value) Line 291: Line 292: def test_too_small(self): Line 293: obj = self.Cls() Line 294: self.assertRaises(ValueError, setattr, obj, "value", -1.0) > Aren't < or > good enough for comparing floats? Update the tests to use very small numbers. Line 295: Line 296: Line 297: class FloatMaxValueTests(VdsmTestCase): Line 298: -- To view, visit https://gerrit.ovirt.org/40822 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I62175172dddce7f84319e3c8669747994e06a697 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir SofferGerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Jenkins CI RO Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: properties: Add properties module
Nir Soffer has posted comments on this change. Change subject: properties: Add properties module .. Patch Set 8: Verified+1 -- To view, visit https://gerrit.ovirt.org/40822 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I62175172dddce7f84319e3c8669747994e06a697 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir SofferGerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Jenkins CI RO Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: properties: Add properties module
gerrit-hooks has posted comments on this change. Change subject: properties: Add properties module .. Patch Set 8: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/40822 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I62175172dddce7f84319e3c8669747994e06a697 Gerrit-PatchSet: 8 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir SofferGerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Jenkins CI RO Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: properties: Add properties module
gerrit-hooks has posted comments on this change. Change subject: properties: Add properties module .. Patch Set 7: * Update tracker: IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0']) -- To view, visit https://gerrit.ovirt.org/40822 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I62175172dddce7f84319e3c8669747994e06a697 Gerrit-PatchSet: 7 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir SofferGerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Jenkins CI RO Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: properties: Add properties module
Nir Soffer has restored this change. Change subject: properties: Add properties module .. Restored -- To view, visit https://gerrit.ovirt.org/40822 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: restore Gerrit-Change-Id: I62175172dddce7f84319e3c8669747994e06a697 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir SofferGerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Jenkins CI RO Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: gerrit-hooks ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: properties: Add properties module
gerrit-hooks has posted comments on this change. Change subject: properties: Add properties module .. Patch Set 6: * Update tracker: IGNORE, no Bug-Url found -- To view, visit https://gerrit.ovirt.org/40822 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I62175172dddce7f84319e3c8669747994e06a697 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir SofferGerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Jenkins CI RO Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: properties: Add properties module
Jenkins CI RO has abandoned this change. Change subject: properties: Add properties module .. Abandoned Abandoned due to no activity - please restore if still relevant -- To view, visit https://gerrit.ovirt.org/40822 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: abandon Gerrit-Change-Id: I62175172dddce7f84319e3c8669747994e06a697 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir SofferGerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Jenkins CI RO Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: gerrit-hooks ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/admin/lists/vdsm-patches@lists.fedorahosted.org
Change in vdsm[master]: properties: Add properties module
Piotr Kliczewski has posted comments on this change. Change subject: properties: Add properties module .. Patch Set 6: (1 comment) https://gerrit.ovirt.org/#/c/40822/6/lib/vdsm/properties.py File lib/vdsm/properties.py: Line 141: class _Number(Property): Line 142: Line 143: def __init__(self, required=False, default=None, doc=None, minval=None, Line 144: maxval=None): Line 145: if minval is not None and default is not None and default < minval: We have bunch of the same checks. It would be good to have single method which would be used in all 4 places. Line 146: raise ValueError("Invalid default %s < %s" % (default, minval)) Line 147: if maxval is not None and default is not None and default > maxval: Line 148: raise ValueError("Invalid default %s > %s" % (default, maxval)) Line 149: super(_Number, self).__init__(required=required, default=default, -- To view, visit https://gerrit.ovirt.org/40822 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I62175172dddce7f84319e3c8669747994e06a697 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir SofferGerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: properties: Add properties module
Nir Soffer has posted comments on this change. Change subject: properties: Add properties module .. Patch Set 6: (1 comment) https://gerrit.ovirt.org/#/c/40822/6/lib/vdsm/properties.py File lib/vdsm/properties.py: Line 141: class _Number(Property): Line 142: Line 143: def __init__(self, required=False, default=None, doc=None, minval=None, Line 144: maxval=None): Line 145: if minval is not None and default is not None and default < minval: > We have bunch of the same checks. It would be good to have single method wh I see 2 places - checking minval and maxval with default. This check is very similar but has different check (< or >) and different log message. Can you show me an example of doing this check in a more clear way? Line 146: raise ValueError("Invalid default %s < %s" % (default, minval)) Line 147: if maxval is not None and default is not None and default > maxval: Line 148: raise ValueError("Invalid default %s > %s" % (default, maxval)) Line 149: super(_Number, self).__init__(required=required, default=default, -- To view, visit https://gerrit.ovirt.org/40822 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I62175172dddce7f84319e3c8669747994e06a697 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir SofferGerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: properties: Add properties module
Nir Soffer has posted comments on this change. Change subject: properties: Add properties module .. Patch Set 6: (8 comments) https://gerrit.ovirt.org/#/c/40822/6/lib/vdsm/properties.py File lib/vdsm/properties.py: Line 119: if not required and default not in values: Line 120: raise ValueError("Default value %s not in allowed values %s" % Line 121: (default, values)) Line 122: super(Enum, self).__init__(required=required, default=default, doc=doc) Line 123: self.values = values Check that all values of of the same type. Line 124: Line 125: def validate(self, value): Line 126: if value not in self.values: Line 127: raise ValueError("Invalid value %r for property %s" % Line 172: Line 173: class Float(_Number): Line 174: Line 175: def validate(self, value): Line 176: if not isinstance(value, float): Should accept also integers: if not isinstance(value, (float, int)): ... Line 177: raise ValueError("Invalid value %r for float property %s" % Line 178: (value, self.name)) Line 179: return super(Float, self).validate(value) Line 180: https://gerrit.ovirt.org/#/c/40822/6/tests/propertiesTests.py File tests/propertiesTests.py: Line 74: self.value = value Line 75: Line 76: def test_required(self): Line 77: # Required property set to None is considered unset Line 78: self.assertRaises(ValueError, self.Cls) > Should we add a test like so? Probably yes. Line 79: Line 80: Line 81: class PropertyDefaultTests(VdsmTestCase): Line 82: Line 112: obj = self.Cls() Line 113: self.assertEqual(obj.value, "2") Line 114: Line 115: def test_allowed(self): Line 116: for value in ("1", "2", "3"): > Can an enum have values of mixed types (eg. [1, "2", u"3"]) ? I know it's I guess we cannot and should not support this. We need more tests. Line 117: obj = self.Cls() Line 118: obj.value = value Line 119: self.assertEqual(obj.value, value) Line 120: Line 121: def test_forbidden(self): Line 122: obj = self.Cls() Line 123: self.assertRaises(ValueError, setattr, obj, "value", "4") Line 124: Line 125: def test_none(self): > So None is always allowed even if it was not specified in the values list? I think I changed it to make it easier to handle missing value in a dict as unset value: class Foo: value = properties.Integer() def __init__(self, d): self.value = d.get("foo") You don't want to do: if "foo" in d: self.value = d["foo"] Properties exists to prevent this boilerplate. Line 126: obj = self.Cls() Line 127: obj.value = None Line 128: self.assertEqual(None, obj.value) Line 129: Line 270: self.assertEqual(obj.value, 3.14) Line 271: Line 272: def test_invalid(self): Line 273: obj = self.Cls() Line 274: self.assertRaises(ValueError, setattr, obj, "value", "not a float") > Are integers allowed? ie. 3 ? It would be nice to allow 3 without needing No, I check for float. I'm not big fan of type automatic type conversion, but treating integers as floats should be safe and should be added. Need to add a test. Line 275: Line 276: def test_none(self): Line 277: obj = self.Cls() Line 278: obj.value = None Line 290: self.assertEqual(1.0, obj.value) Line 291: Line 292: def test_too_small(self): Line 293: obj = self.Cls() Line 294: self.assertRaises(ValueError, setattr, obj, "value", -1.0) > You might want to test with a value that is really, really close to the min Aren't < or > good enough for comparing floats? See the _Number class in properties.py Line 295: Line 296: Line 297: class FloatMaxValueTests(VdsmTestCase): Line 298: Line 350: Line 351: def test_none(self): Line 352: obj = self.Cls() Line 353: obj.value = None Line 354: self.assertEqual(None, obj.value) > Test with 0, 1, [], {}, and other costructs that are implicitly interpreted Anything which is not True or False will raise ValueError, see test_invalid. But yes, we need more tests, maybe with permutations. Line 355: Line 356: Line 357: class BooleanDefaultTests(VdsmTestCase): Line 358: -- To view, visit https://gerrit.ovirt.org/40822 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I62175172dddce7f84319e3c8669747994e06a697 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir SofferGerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Federico
Change in vdsm[master]: properties: Add properties module
Adam Litke has posted comments on this change. Change subject: properties: Add properties module .. Patch Set 6: (6 comments) Very nice! https://gerrit.ovirt.org/#/c/40822/6/tests/propertiesTests.py File tests/propertiesTests.py: Line 74: self.value = value Line 75: Line 76: def test_required(self): Line 77: # Required property set to None is considered unset Line 78: self.assertRaises(ValueError, self.Cls) Should we add a test like so? def test_required_set_none(self): obj = self.Cls(5) self.assertRaises(ValueError, setattr, obj, 'value', None) Line 79: Line 80: Line 81: class PropertyDefaultTests(VdsmTestCase): Line 82: Line 112: obj = self.Cls() Line 113: self.assertEqual(obj.value, "2") Line 114: Line 115: def test_allowed(self): Line 116: for value in ("1", "2", "3"): Can an enum have values of mixed types (eg. [1, "2", u"3"]) ? I know it's not a great idea to do this. Line 117: obj = self.Cls() Line 118: obj.value = value Line 119: self.assertEqual(obj.value, value) Line 120: Line 121: def test_forbidden(self): Line 122: obj = self.Cls() Line 123: self.assertRaises(ValueError, setattr, obj, "value", "4") Line 124: Line 125: def test_none(self): So None is always allowed even if it was not specified in the values list? I wonder if we want to treat None like any other value and if it's not in the list it cannot be set. Line 126: obj = self.Cls() Line 127: obj.value = None Line 128: self.assertEqual(None, obj.value) Line 129: Line 270: self.assertEqual(obj.value, 3.14) Line 271: Line 272: def test_invalid(self): Line 273: obj = self.Cls() Line 274: self.assertRaises(ValueError, setattr, obj, "value", "not a float") Are integers allowed? ie. 3 ? It would be nice to allow 3 without needing to write 3.0. Line 275: Line 276: def test_none(self): Line 277: obj = self.Cls() Line 278: obj.value = None Line 290: self.assertEqual(1.0, obj.value) Line 291: Line 292: def test_too_small(self): Line 293: obj = self.Cls() Line 294: self.assertRaises(ValueError, setattr, obj, "value", -1.0) You might want to test with a value that is really, really close to the minimum value (ie. 0.001). Line 295: Line 296: Line 297: class FloatMaxValueTests(VdsmTestCase): Line 298: Line 350: Line 351: def test_none(self): Line 352: obj = self.Cls() Line 353: obj.value = None Line 354: self.assertEqual(None, obj.value) Test with 0, 1, [], {}, and other costructs that are implicitly interpreted as boolean values. Line 355: Line 356: Line 357: class BooleanDefaultTests(VdsmTestCase): Line 358: -- To view, visit https://gerrit.ovirt.org/40822 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I62175172dddce7f84319e3c8669747994e06a697 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir SofferGerrit-Reviewer: Adam Litke Gerrit-Reviewer: Allon Mureinik Gerrit-Reviewer: Dan Kenigsberg Gerrit-Reviewer: Daniel Erez Gerrit-Reviewer: Federico Simoncelli Gerrit-Reviewer: Francesco Romani Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer Gerrit-Reviewer: Piotr Kliczewski Gerrit-Reviewer: Yaniv Bronhaim Gerrit-Reviewer: gerrit-hooks Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: properties: Add properties module
automat...@ovirt.org has posted comments on this change. Change subject: properties: Add properties module .. Patch Set 5: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/40822 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I62175172dddce7f84319e3c8669747994e06a697 Gerrit-PatchSet: 5 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Daniel Erez de...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: properties: Add properties module
automat...@ovirt.org has posted comments on this change. Change subject: properties: Add properties module .. Patch Set 6: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/40822 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I62175172dddce7f84319e3c8669747994e06a697 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Daniel Erez de...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: properties: Add properties module
Nir Soffer has posted comments on this change. Change subject: properties: Add properties module .. Patch Set 6: Verified+1 This version addresses Francesco and Daniel comments. -- To view, visit https://gerrit.ovirt.org/40822 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I62175172dddce7f84319e3c8669747994e06a697 Gerrit-PatchSet: 6 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Daniel Erez de...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: properties: Add properties module
automat...@ovirt.org has posted comments on this change. Change subject: properties: Add properties module .. Patch Set 4: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/40822 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I62175172dddce7f84319e3c8669747994e06a697 Gerrit-PatchSet: 4 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Daniel Erez de...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: properties: Add properties module
automat...@ovirt.org has posted comments on this change. Change subject: properties: Add properties module .. Patch Set 3: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/40822 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I62175172dddce7f84319e3c8669747994e06a697 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: properties: Add properties module
automat...@ovirt.org has posted comments on this change. Change subject: properties: Add properties module .. Patch Set 2: * Update tracker::IGNORE, no Bug-Url found * Check Bug-Url::WARN, no bug url found, make sure header matches 'Bug-Url: ' and is a valid url. * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.5', 'ovirt-3.4', 'ovirt-3.3']) -- To view, visit https://gerrit.ovirt.org/40822 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I62175172dddce7f84319e3c8669747994e06a697 Gerrit-PatchSet: 2 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: No ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: properties: Add properties module
Daniel Erez has posted comments on this change. Change subject: properties: Add properties module .. Patch Set 3: (1 comment) https://gerrit.ovirt.org/#/c/40822/3/lib/vdsm/properties.py File lib/vdsm/properties.py: Line 2: reusable add to Makefile/vdsm.spec/vdsm-python/... -- To view, visit https://gerrit.ovirt.org/40822 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I62175172dddce7f84319e3c8669747994e06a697 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Daniel Erez de...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: properties: Add properties module
Francesco Romani has posted comments on this change. Change subject: properties: Add properties module .. Patch Set 3: Code-Review+1 (2 comments) yes, this is cool. It reminds me of definitions of columns in Model classes in ORM, but even nicer. This can solve the abuse of __slots__ we are doing in device classes. Very nicely done, I really like it. The only minor concern is if the 'properties' is really a suitable name. Seems a bit too generic, and I don't have any really better suggestion. https://gerrit.ovirt.org/#/c/40822/3/lib/vdsm/properties.py File lib/vdsm/properties.py: Line 1: missing license boilerplate. I don't really mind, but legal can :) Line 2: properties - reusable properties Line 3: Line 4: Properties are reusable objects similar to builtin property() function, adding Line 5: input validation and initialization grantees. Line 1: Line 2: properties - reusable properties Line 3: Line 4: Properties are reusable objects similar to builtin property() function, adding Line 5: input validation and initialization grantees. guarantees? Line 6: Line 7: Line 8: This module provides the following properties: Line 9: -- To view, visit https://gerrit.ovirt.org/40822 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I62175172dddce7f84319e3c8669747994e06a697 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Daniel Erez de...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
Change in vdsm[master]: properties: Add properties module
Nir Soffer has posted comments on this change. Change subject: properties: Add properties module .. Patch Set 3: (3 comments) About the name, this is mainly about input validation - so maybe validation.py? https://gerrit.ovirt.org/#/c/40822/3/lib/vdsm/properties.py File lib/vdsm/properties.py: Line 1: missing license boilerplate. I don't really mind, but legal can :) Will add when this becomes real Line 2: properties - reusable properties Line 3: Line 4: Properties are reusable objects similar to builtin property() function, adding Line 5: input validation and initialization grantees. Line 1: Line 2: properties - reusable properties add to Makefile/vdsm.spec/vdsm-python/... Sure, when time comes Line 3: Line 4: Properties are reusable objects similar to builtin property() function, adding Line 5: input validation and initialization grantees. Line 6: Line 1: Line 2: properties - reusable properties Line 3: Line 4: Properties are reusable objects similar to builtin property() function, adding Line 5: input validation and initialization grantees. guarantees? Will fix Line 6: Line 7: Line 8: This module provides the following properties: Line 9: -- To view, visit https://gerrit.ovirt.org/40822 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I62175172dddce7f84319e3c8669747994e06a697 Gerrit-PatchSet: 3 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Allon Mureinik amure...@redhat.com Gerrit-Reviewer: Dan Kenigsberg dan...@redhat.com Gerrit-Reviewer: Daniel Erez de...@redhat.com Gerrit-Reviewer: Federico Simoncelli fsimo...@redhat.com Gerrit-Reviewer: Francesco Romani from...@redhat.com Gerrit-Reviewer: Jenkins CI Gerrit-Reviewer: Nir Soffer nsof...@redhat.com Gerrit-Reviewer: Piotr Kliczewski piotr.kliczew...@gmail.com Gerrit-Reviewer: Yaniv Bronhaim ybron...@redhat.com Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes ___ vdsm-patches mailing list vdsm-patches@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches