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é