On Fri, Sep 9, 2011 at 15:21, Michael Hanselmann <[email protected]> wrote: > Am 9. September 2011 13:46 schrieb René Nussbaumer <[email protected]>: >> On Fri, Sep 9, 2011 at 13:14, Michael Hanselmann <[email protected]> wrote: >>> --- a/lib/rapi/baserlib.py >>> +++ b/lib/rapi/baserlib.py >>> +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. > > See interdiff at the end. > >>> +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. > > Disagreed. If you click through the generated docs it's easier to find > the base class. I added a reference to the metaclass, though. > >>> --- a/test/ganeti.rapi.baserlib_unittest.py >>> +++ b/test/ganeti.rapi.baserlib_unittest.py >>> +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) > > Nope, this one tests whether setting “None” as the opcode also raises > an exception when the HTTP method's direct handler is also defined. > >>> + 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? > > All other RAPI unittests (see the bazillion patches) are working sets. > > Michael >
Thanks, LGTM! René
