[Freeipa-devel] Plugin registration API (Was: [PATCH] Return klass in api.register())

[Nathaniel McCallum]

On Tue, 2013-07-30 at 18:58 +0200, Petr Viktorin wrote:
> On 07/30/2013 05:16 PM, Nathaniel McCallum wrote:
> > From 371e080fb6a86da6f0ec574c8cbc0d8d06c18c99 Mon Sep 17 00:00:00 2001
> > From: Nathaniel McCallum<npmccallum at redhat.com>
> > Date: Tue, 30 Jul 2013 11:13:55 -0400
> > Subject: [PATCH] Return klass in api.register()
> >
> > This permits the use of api.register as a decorator.
> 
> Thanks for the patch.
> Was there any discussion/reason for this? Can you file a ticket?
> I'm not opposed, I agree this would allow less typing for plugin authors.

There was not really any discussion. I just simply noticed that a single
line change would make plugins a bit more pythonic. I mentioned it to
rcrit and he didn't see any problem with it. So I wrote the patch.

Expressing registration as a decorator has numerous advantages,
including being less error prone and easier to read. But typing less is
probably a good enough reason. :)

In my mind, this can be merged now since it doesn't break any backwards
compatibility and enables a syntax that is easier to modify in the
future. It is the migration from a function to a decorator that is where
all the work lies. Once you have decorators, you can fairly easily do a
sed replacement (or maintain backwards compatibility if you like).

> I also think the plugin registration API needs a larger redesign, though 
> I haven't shared my thoughts on it lately as there are more pressing 
> things to do. I'll correct that now that there's (sort of) a thread on 
> the topic :)
> 
> Basically, my gripe is that plugin registration only works on the one 
> global API object, ipalib.api. If you create a different API object, you 
> can't register plugins on it. Different API object would be useful for 
> testing, or for running IPA commands on another master (there's an ugly 
> hack in ipa-replica-install that does that).
> 
> A rough sketch of what I had in mind would be:
> 
> plugin.py:
>      # the "register" decorator will collect registered items;
>      # it must be named "register" so the loading routine finds it
>      register = plugable.Registry()
> 
>      @register
>      class obj_mod(LDAPUpdate):
>          ....
> 
> plugable.py:
>      class Registry:
>          def __call__(self, item):
>              self.items.append(item)
> 
>      class API:
>          def load_plugin_package(self, package_name):
>              plugin = __import__(package_name).register
>              for item in register.items:
>                  self.register(item)
> 
> Related ticket for this: https://fedorahosted.org/freeipa/ticket/3090
> 
> 
> Maybe if we're giving an incentive for plugin authors to rewrite 
> plugins, we should give them an API that's at least compatible with the 
> above goals, so they won't have to rewrite again some time later.

Having a registration collector as an independent object makes a lot of
sense. I also prefer type-safety to strings.

Make sure you make the decorator a callable though. This permits you to
easily add options later without breaking backwards compatibility.

You want:
@register()
class foo:
  pass

Not:
@register
class foo:
  pass

Also, you can avoid the creation of a registry object for each plugin.
You can essentially do module cache poisoning to force an alternative
registry. This saves some typing and keeps the alternate code in the
same scope as the tests for that alternate path...

Lastly, you could use class hierarchy introspection to avoid
registration altogether. This has the advantage of some natural type
safety.

Nathaniel