Change in vdsm[master]: properties: Add properties module

2016-07-11 Thread automation
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 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 
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

2016-07-11 Thread alitke
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 Soffer 
Reviewed-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

2016-07-11 Thread alitke
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 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 
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

2016-07-11 Thread alitke
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 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 
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

2016-07-08 Thread nsoffer
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 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 
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

2016-07-08 Thread automation
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 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 
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

2016-07-08 Thread nsoffer
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 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 
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

2016-07-08 Thread nsoffer
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 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 
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

2016-07-08 Thread automation
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 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 
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

2016-07-08 Thread automation
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 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 
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

2016-07-03 Thread nsoffer
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 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

2016-07-02 Thread automation
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 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 
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

2016-07-02 Thread Jenkins CI RO
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 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

2015-12-11 Thread piotr . kliczewski
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 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: 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

2015-12-11 Thread nsoffer
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 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: 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

2015-12-10 Thread nsoffer
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 Soffer 
Gerrit-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

2015-12-10 Thread alitke
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 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: 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

2015-05-22 Thread automation
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

2015-05-22 Thread automation
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

2015-05-22 Thread nsoffer
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

2015-05-22 Thread automation
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

2015-05-18 Thread automation
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

2015-05-18 Thread automation
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

2015-05-18 Thread derez
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

2015-05-18 Thread fromani
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

2015-05-18 Thread nsoffer
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