LGTM

Thanks,

Guido


On Fri, Feb 15, 2013 at 1:01 PM, Iustin Pop <[email protected]> wrote:
>
> commit 36e1c7e840711b30fc60fd44cb0e47e4ee5c1133
> Merge: d9a2252 473d87a
> Author: Iustin Pop <[email protected]>
> Date:   Fri Feb 15 12:53:30 2013 +0100
>
>     Merge branch 'devel-2.7'
>
>     * devel-2.7:
>       Rename lib/objectutils to outils.py
>       Fix typo in gnt-group manpage
>       Fix wrong type in a docstring of the RAPI subsystem
>       Finish the remote→restricted commands rename
>       Enable use of the priority option in hbal
>       Add CLI-level option to override the priority
>       Add functions to parse CLI-level format of priorities
>       Add a function to change an OpCode's priority
>       Make hbal opcode annotation more generic
>       Add unit tests for RADOSBLockDevice
>       Fix rbd showmapped output parsing
>       Change default xen root path to /dev/xvda1
>       Removes check for conflicts from NetworkDisconnect
>       If _UnlockedLookupNetwork() fails raise error
>       Force conflicts check in LUNetworkDisconnect
>
>     Also updated objects.py with more outils renames.
>
>     Signed-off-by: Iustin Pop <[email protected]>
>
> diff --cc lib/objects.py
> index 6340d8d,588ef03..fa811a6
> --- a/lib/objects.py
> +++ b/lib/objects.py
> @@@ -406,7 -447,7 +406,7 @@@ class ConfigData(ConfigObject)
>       mydict = super(ConfigData, self).ToDict()
>       mydict["cluster"] = mydict["cluster"].ToDict()
>       for key in "nodes", "instances", "nodegroups", "networks":
> -       mydict[key] = objectutils.ContainerToDicts(mydict[key])
>  -      mydict[key] = self._ContainerToDicts(mydict[key])
> ++      mydict[key] = outils.ContainerToDicts(mydict[key])
>
>       return mydict
>
> @@@ -417,12 -458,10 +417,12 @@@
>       """
>       obj = super(ConfigData, cls).FromDict(val)
>       obj.cluster = Cluster.FromDict(obj.cluster)
> -     obj.nodes = objectutils.ContainerFromDicts(obj.nodes, dict, Node)
>  -    obj.nodes = cls._ContainerFromDicts(obj.nodes, dict, Node)
>  -    obj.instances = cls._ContainerFromDicts(obj.instances, dict, Instance)
>  -    obj.nodegroups = cls._ContainerFromDicts(obj.nodegroups, dict, 
> NodeGroup)
>  -    obj.networks = cls._ContainerFromDicts(obj.networks, dict, Network)
> ++    obj.nodes = outils.ContainerFromDicts(obj.nodes, dict, Node)
>  +    obj.instances = \
> -       objectutils.ContainerFromDicts(obj.instances, dict, Instance)
> ++      outils.ContainerFromDicts(obj.instances, dict, Instance)
>  +    obj.nodegroups = \
> -       objectutils.ContainerFromDicts(obj.nodegroups, dict, NodeGroup)
> -     obj.networks = objectutils.ContainerFromDicts(obj.networks, dict, 
> Network)
> ++      outils.ContainerFromDicts(obj.nodegroups, dict, NodeGroup)
> ++    obj.networks = outils.ContainerFromDicts(obj.networks, dict, Network)
>       return obj
>
>     def HasAnyDiskOfType(self, dev_type):
> @@@ -732,7 -771,7 +732,7 @@@ class Disk(ConfigObject)
>       for attr in ("children",):
>         alist = bo.get(attr, None)
>         if alist:
> -         bo[attr] = objectutils.ContainerToDicts(alist)
>  -        bo[attr] = self._ContainerToDicts(alist)
> ++        bo[attr] = outils.ContainerToDicts(alist)
>       return bo
>
>     @classmethod
> @@@ -742,7 -781,7 +742,7 @@@
>       """
>       obj = super(Disk, cls).FromDict(val)
>       if obj.children:
> -       obj.children = objectutils.ContainerFromDicts(obj.children, list, 
> Disk)
>  -      obj.children = cls._ContainerFromDicts(obj.children, list, Disk)
> ++      obj.children = outils.ContainerFromDicts(obj.children, list, Disk)
>       if obj.logical_id and isinstance(obj.logical_id, list):
>         obj.logical_id = tuple(obj.logical_id)
>       if obj.physical_id and isinstance(obj.physical_id, list):
> @@@ -1100,7 -1139,7 +1100,7 @@@ class Instance(TaggableObject)
>       for attr in "nics", "disks":
>         alist = bo.get(attr, None)
>         if alist:
> -         nlist = objectutils.ContainerToDicts(alist)
>  -        nlist = self._ContainerToDicts(alist)
> ++        nlist = outils.ContainerToDicts(alist)
>         else:
>           nlist = []
>         bo[attr] = nlist
> @@@ -1119,8 -1158,8 +1119,8 @@@
>       if "admin_up" in val:
>         del val["admin_up"]
>       obj = super(Instance, cls).FromDict(val)
> -     obj.nics = objectutils.ContainerFromDicts(obj.nics, list, NIC)
> -     obj.disks = objectutils.ContainerFromDicts(obj.disks, list, Disk)
>  -    obj.nics = cls._ContainerFromDicts(obj.nics, list, NIC)
>  -    obj.disks = cls._ContainerFromDicts(obj.disks, list, Disk)
> ++    obj.nics = outils.ContainerFromDicts(obj.nics, list, NIC)
> ++    obj.disks = outils.ContainerFromDicts(obj.disks, list, Disk)
>       return obj
>
>     def UpgradeConfig(self):
> @@@ -1314,12 -1353,12 +1314,12 @@@ class Node(TaggableObject)
>
>       hv_state = data.get("hv_state", None)
>       if hv_state is not None:
> -       data["hv_state"] = objectutils.ContainerToDicts(hv_state)
>  -      data["hv_state"] = self._ContainerToDicts(hv_state)
> ++      data["hv_state"] = outils.ContainerToDicts(hv_state)
>
>       disk_state = data.get("disk_state", None)
>       if disk_state is not None:
>         data["disk_state"] = \
> -         dict((key, objectutils.ContainerToDicts(value))
>  -        dict((key, self._ContainerToDicts(value))
> ++        dict((key, outils.ContainerToDicts(value))
>                for (key, value) in disk_state.items())
>
>       return data
> @@@ -1332,12 -1371,11 +1332,12 @@@
>       obj = super(Node, cls).FromDict(val)
>
>       if obj.hv_state is not None:
>  -      obj.hv_state = cls._ContainerFromDicts(obj.hv_state, dict, 
> NodeHvState)
>  +      obj.hv_state = \
> -         objectutils.ContainerFromDicts(obj.hv_state, dict, NodeHvState)
> ++        outils.ContainerFromDicts(obj.hv_state, dict, NodeHvState)
>
>       if obj.disk_state is not None:
>         obj.disk_state = \
> -         dict((key, objectutils.ContainerFromDicts(value, dict, 
> NodeDiskState))
>  -        dict((key, cls._ContainerFromDicts(value, dict, NodeDiskState))
> ++        dict((key, outils.ContainerFromDicts(value, dict, NodeDiskState))
>                for (key, value) in obj.disk_state.items())
>
>       return obj
> @@@ -1906,7 -1933,7 +1906,7 @@@ class _QueryResponseBase(ConfigObject)
>
>       """
>       mydict = super(_QueryResponseBase, self).ToDict()
> -     mydict["fields"] = objectutils.ContainerToDicts(mydict["fields"])
>  -    mydict["fields"] = self._ContainerToDicts(mydict["fields"])
> ++    mydict["fields"] = outils.ContainerToDicts(mydict["fields"])
>       return mydict
>
>     @classmethod
> @@@ -1915,8 -1942,7 +1915,8 @@@
>
>       """
>       obj = super(_QueryResponseBase, cls).FromDict(val)
>  -    obj.fields = cls._ContainerFromDicts(obj.fields, list, 
> QueryFieldDefinition)
>  +    obj.fields = \
> -       objectutils.ContainerFromDicts(obj.fields, list, QueryFieldDefinition)
> ++      outils.ContainerFromDicts(obj.fields, list, QueryFieldDefinition)
>       return obj
>
>
> diff --cc lib/outils.py
> index 0000000,e2742b1..f0f6558
> mode 000000,100644..100644
> --- a/lib/outils.py
> +++ b/lib/outils.py
> @@@ -1,0 -1,93 +1,150 @@@
> + #
> + #
> +
> + # Copyright (C) 2012 Google Inc.
> + #
> + # This program is free software; you can redistribute it and/or modify
> + # it under the terms of the GNU General Public License as published by
> + # the Free Software Foundation; either version 2 of the License, or
> + # (at your option) any later version.
> + #
> + # This program is distributed in the hope that it will be useful, but
> + # WITHOUT ANY WARRANTY; without even the implied warranty of
> + # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + # General Public License for more details.
> + #
> + # You should have received a copy of the GNU General Public License
> + # along with this program; if not, write to the Free Software
> + # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> + # 02110-1301, USA.
> +
> + """Module for object related utils."""
> +
> +
> ++#: Supported container types for serialization/de-serialization (must be a
> ++#: tuple as it's used as a parameter for C{isinstance})
> ++_SEQUENCE_TYPES = (list, tuple, set, frozenset)
> ++
> ++
> + class AutoSlots(type):
> +   """Meta base class for __slots__ definitions.
> +
> +   """
> +   def __new__(mcs, name, bases, attrs):
> +     """Called when a class should be created.
> +
> +     @param mcs: The meta class
> +     @param name: Name of created class
> +     @param bases: Base classes
> +     @type attrs: dict
> +     @param attrs: Class attributes
> +
> +     """
> +     assert "__slots__" not in attrs, \
> +       "Class '%s' defines __slots__ when it should not" % name
> +
> +     attrs["__slots__"] = mcs._GetSlots(attrs)
> +
> +     return type.__new__(mcs, name, bases, attrs)
> +
> +   @classmethod
> +   def _GetSlots(mcs, attrs):
> +     """Used to get the list of defined slots.
> +
> +     @param attrs: The attributes of the class
> +
> +     """
> +     raise NotImplementedError
> +
> +
> + class ValidatedSlots(object):
> +   """Sets and validates slots.
> +
> +   """
> +   __slots__ = []
> +
> +   def __init__(self, **kwargs):
> +     """Constructor for BaseOpCode.
> +
> +     The constructor takes only keyword arguments and will set
> +     attributes on this object based on the passed arguments. As such,
> +     it means that you should not pass arguments which are not in the
> +     __slots__ attribute for this class.
> +
> +     """
> +     slots = self.GetAllSlots()
> +     for (key, value) in kwargs.items():
> +       if key not in slots:
> +         raise TypeError("Object %s doesn't support the parameter '%s'" %
> +                         (self.__class__.__name__, key))
> +       setattr(self, key, value)
> +
> +   @classmethod
> +   def GetAllSlots(cls):
> +     """Compute the list of all declared slots for a class.
> +
> +     """
> +     slots = []
> +     for parent in cls.__mro__:
> +       slots.extend(getattr(parent, "__slots__", []))
> +     return slots
> +
> +   def Validate(self):
> +     """Validates the slots.
> +
> +     This method must be implemented by the child classes.
> +
> +     """
> +     raise NotImplementedError
> ++
> ++
> ++def ContainerToDicts(container):
> ++  """Convert the elements of a container to standard Python types.
> ++
> ++  This method converts a container with elements to standard Python types. 
> If
> ++  the input container is of the type C{dict}, only its values are touched.
> ++  Those values, as well as all elements of input sequences, must support a
> ++  C{ToDict} method returning a serialized version.
> ++
> ++  @type container: dict or sequence (see L{_SEQUENCE_TYPES})
> ++
> ++  """
> ++  if isinstance(container, dict):
> ++    ret = dict([(k, v.ToDict()) for k, v in container.items()])
> ++  elif isinstance(container, _SEQUENCE_TYPES):
> ++    ret = [elem.ToDict() for elem in container]
> ++  else:
> ++    raise TypeError("Unknown container type '%s'" % type(container))
> ++
> ++  return ret
> ++
> ++
> ++def ContainerFromDicts(source, c_type, e_type):
> ++  """Convert a container from standard python types.
> ++
> ++  This method converts a container with standard Python types to objects. If
> ++  the container is a dict, we don't touch the keys, only the values.
> ++
> ++  @type source: None, dict or sequence (see L{_SEQUENCE_TYPES})
> ++  @param source: Input data
> ++  @type c_type: type class
> ++  @param c_type: Desired type for returned container
> ++  @type e_type: element type class
> ++  @param e_type: Item type for elements in returned container (must have a
> ++    C{FromDict} class method)
> ++
> ++  """
> ++  if not isinstance(c_type, type):
> ++    raise TypeError("Container type '%s' is not a type" % type(c_type))
> ++
> ++  if source is None:
> ++    source = c_type()
> ++
> ++  if c_type is dict:
> ++    ret = dict([(k, e_type.FromDict(v)) for k, v in source.items()])
> ++  elif c_type in _SEQUENCE_TYPES:
> ++    ret = c_type(map(e_type.FromDict, source))
> ++  else:
> ++    raise TypeError("Unknown container type '%s'" % c_type)
> ++
> ++  return ret
> diff --cc test/py/ganeti.outils_unittest.py
> index 0000000,93f5212..22d2177
> mode 000000,100755..100755
> --- a/test/py/ganeti.outils_unittest.py
> +++ b/test/py/ganeti.outils_unittest.py
> @@@ -1,0 -1,50 +1,107 @@@
> + #!/usr/bin/python
> + #
> +
> + # Copyright (C) 2012, 2013 Google Inc.
> + #
> + # This program is free software; you can redistribute it and/or modify
> + # it under the terms of the GNU General Public License as published by
> + # the Free Software Foundation; either version 2 of the License, or
> + # (at your option) any later version.
> + #
> + # This program is distributed in the hope that it will be useful, but
> + # WITHOUT ANY WARRANTY; without even the implied warranty of
> + # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + # General Public License for more details.
> + #
> + # You should have received a copy of the GNU General Public License
> + # along with this program; if not, write to the Free Software
> + # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> + # 02110-1301, USA.
> +
> +
> + """Script for unittesting the outils module"""
> +
> +
> + import unittest
> +
> + from ganeti import outils
> +
> + import testutils
> +
> +
> + class SlotsAutoSlot(outils.AutoSlots):
> +   @classmethod
> +   def _GetSlots(mcs, attr):
> +     return attr["SLOTS"]
> +
> +
> + class AutoSlotted(object):
> +   __metaclass__ = SlotsAutoSlot
> +
> +   SLOTS = ["foo", "bar", "baz"]
> +
> +
> + class TestAutoSlot(unittest.TestCase):
> +   def test(self):
> +     slotted = AutoSlotted()
> +     self.assertEqual(slotted.__slots__, AutoSlotted.SLOTS)
> +
> ++
> ++class TestContainerToDicts(unittest.TestCase):
> ++  def testUnknownType(self):
> ++    for value in [None, 19410, "xyz"]:
> ++      try:
> ++        outils.ContainerToDicts(value)
> ++      except TypeError, err:
> ++        self.assertTrue(str(err).startswith("Unknown container type"))
> ++      else:
> ++        self.fail("Exception was not raised")
> ++
> ++  def testEmptyDict(self):
> ++    value = {}
> ++    self.assertFalse(type(value) in outils._SEQUENCE_TYPES)
> ++    self.assertEqual(outils.ContainerToDicts(value), {})
> ++
> ++  def testEmptySequences(self):
> ++    for cls in [list, tuple, set, frozenset]:
> ++      self.assertEqual(outils.ContainerToDicts(cls()), [])
> ++
> ++
> ++class _FakeWithFromDict:
> ++  def FromDict(self, _):
> ++    raise NotImplemented
> ++
> ++
> ++class TestContainerFromDicts(unittest.TestCase):
> ++  def testUnknownType(self):
> ++    for cls in [str, int, bool]:
> ++      try:
> ++        outils.ContainerFromDicts(None, cls, NotImplemented)
> ++      except TypeError, err:
> ++        self.assertTrue(str(err).startswith("Unknown container type"))
> ++      else:
> ++        self.fail("Exception was not raised")
> ++
> ++      try:
> ++        outils.ContainerFromDicts(None, cls(), NotImplemented)
> ++      except TypeError, err:
> ++        self.assertTrue(str(err).endswith("is not a type"))
> ++      else:
> ++        self.fail("Exception was not raised")
> ++
> ++  def testEmptyDict(self):
> ++    value = {}
> ++    self.assertFalse(type(value) in outils._SEQUENCE_TYPES)
> ++    self.assertEqual(outils.ContainerFromDicts(value, dict,
> ++                                                    NotImplemented),
> ++                     {})
> ++
> ++  def testEmptySequences(self):
> ++    for cls in [list, tuple, set, frozenset]:
> ++      self.assertEqual(outils.ContainerFromDicts([], cls,
> ++                                                      _FakeWithFromDict),
> ++                       cls())
> ++
> ++
> + if __name__ == "__main__":
> +   testutils.GanetiTestProgram()
>
> --
> thanks,
> iustin



-- 
Guido Trotter
Ganeti engineering
Google Germany

Reply via email to