On Fri, Sep 9, 2011 at 13:14, Michael Hanselmann <[email protected]> wrote:
> This base class, which employs a meta class for the actual work, allows
> easier definitions of RAPI resources using opcodes. Follow-up patches
> will change some of the existing RAPI resources.
>
> The long-term goal with these changes is to make it easier to verify the
> consistency of RAPI with CLI (or the underlying opcodes).
>
> Signed-off-by: Michael Hanselmann <[email protected]>
> ---
>  lib/rapi/baserlib.py                  |   63 
> +++++++++++++++++++++++++++++++++
>  test/ganeti.rapi.baserlib_unittest.py |   32 +++++++++++++++++
>  2 files changed, 95 insertions(+), 0 deletions(-)
>
> diff --git a/lib/rapi/baserlib.py b/lib/rapi/baserlib.py
> index 8847f77..cc322c2 100644
> --- a/lib/rapi/baserlib.py
> +++ b/lib/rapi/baserlib.py
> @@ -33,11 +33,20 @@ from ganeti import luxi
>  from ganeti import rapi
>  from ganeti import http
>  from ganeti import errors
> +from ganeti import compat
>
>
>  # Dummy value to detect unchanged parameters
>  _DEFAULT = object()
>
> +#: Supported HTTP methods
> +_SUPPORTED_METHODS = frozenset([
> +  http.HTTP_DELETE,
> +  http.HTTP_GET,
> +  http.HTTP_POST,
> +  http.HTTP_PUT,
> +  ])
> +
>
>  def BuildUriList(ids, uri_format, uri_fields=("name", "uri")):
>   """Builds a URI list as used by index resources.
> @@ -391,3 +400,57 @@ class ResourceBase(object):
>     except luxi.TimeoutError, err:
>       raise http.HttpGatewayTimeout("Timeout while talking to the master"
>                                     " daemon: %s" % err)
> +
> +
> +class _MetaOpcodeResource(type):
> +  """Meta class for RAPI resources.
> +
> +  """

Can you please elaborate the changes made by this metaclass to the
underlying class in the docstring? Maybe based on the attributes. Or
what attributes are possible.

> +  _ATTRS = [(method, "%s_OPCODE" % method, "%s_RENAME" % method,
> +             "Get%sOpInput" % method.capitalize())
> +            for method in _SUPPORTED_METHODS]
> +
> +  def __call__(mcs, *args, **kwargs):
> +    """Instantiates class and patches it for use by the RAPI daemon.
> +
> +    """
> +    # Access to private attributes of a client class, pylint: disable=W0212
> +    obj = type.__call__(mcs, *args, **kwargs)
> +
> +    for (method, op_attr, rename_attr, fn_attr) in mcs._ATTRS:
> +      try:
> +        opcode = getattr(obj, op_attr)
> +      except AttributeError:
> +        assert not hasattr(obj, rename_attr)
> +        assert not hasattr(obj, fn_attr)
> +        continue
> +
> +      assert not hasattr(obj, method)
> +
> +      setattr(obj, method,
> +              compat.partial(obj._GenericHandler, opcode,
> +                             getattr(obj, rename_attr, None),
> +                             getattr(obj, fn_attr, obj._GetDefaultData)))
> +
> +    return obj
> +
> +
> +class OpcodeResource(ResourceBase):
> +  """Base class for opcode-based RAPI resources.
> +
> +  Instances of this class automatically gain handler functions for any method
> +  for which a C{$METHOD$_OPCODE} variable is defined at class level. 
> Subclasses
> +  can define a C{Get$Method$OpInput} method to do their own opcode input
> +  processing (e.g. for static values). The C{$METHOD$_RENAME} variable 
> defines
> +  which values are renamed (see L{FillOpCode}).
> +
> +  """

I would move that to the metaclass docstring.

> +  __metaclass__ = _MetaOpcodeResource
> +
> +  def _GetDefaultData(self):
> +    return (self.request_body, None)
> +
> +  def _GenericHandler(self, opcode, rename, fn):
> +    (body, static) = fn()
> +    op = FillOpcode(opcode, body, static, rename=rename)
> +    return self.SubmitJob([op])
> diff --git a/test/ganeti.rapi.baserlib_unittest.py 
> b/test/ganeti.rapi.baserlib_unittest.py
> index fc29cce..6b54eac 100755
> --- a/test/ganeti.rapi.baserlib_unittest.py
> +++ b/test/ganeti.rapi.baserlib_unittest.py
> @@ -97,5 +97,37 @@ class TestFillOpcode(unittest.TestCase):
>                       rename={ "data": "test", })
>
>
> +class TestOpcodeResource(unittest.TestCase):
> +  def testDoubleDefinition(self):
> +    class _TClass(baserlib.OpcodeResource):
> +      GET_OPCODE = opcodes.OpTestDelay
> +      def GET(self): pass
> +
> +    self.assertRaises(AssertionError, _TClass, None, None, None)
> +
> +  def testNoOpCode(self):
> +    class _TClass(baserlib.OpcodeResource):
> +      POST_OPCODE = None
> +      def POST(self): pass
> +
> +    self.assertRaises(AssertionError, _TClass, None, None, None)

Is this not just due to double definition? (See method above)

> +
> +  def testIllegalRename(self):
> +    class _TClass(baserlib.OpcodeResource):
> +      PUT_RENAME = None
> +      def PUT(self): pass
> +
> +    self.assertRaises(AssertionError, _TClass, None, None, None)
> +
> +  def testEmpty(self):
> +    class _Empty(baserlib.OpcodeResource):
> +      pass
> +
> +    obj = _Empty(None, None, None)
> +    for attr in ["GetPostOpInput", "GetPutOpInput", "GetGetOpInput",
> +                 "GetDeleteOpInput"]:
> +      self.assertFalse(hasattr(obj, attr))

Could you also test a working set?

René

Reply via email to