674777 - Add support for abc to load_and_configure

Status: VERIFIED FIXED

(Cloud Services :: Server: Core, defect)

Description

[Toby Elliott [:telliott]]

Something along the lines of 

if foo = get(section.interface):
  a,b = split(foo)
  __import__(a)
  newclass=getattr(a, b)
  instance = newclass()
  if instance.hasattr(get_from_config):
    return newclass.get_from_config(config, section)

That lets us add an effective interface definition that can be optionally added to the config file.

Comment 1

[Tarek Ziadé (:tarek)]

I am not sure to understand the pseudo-code example, can you elaborate with a configuration file example ?

Comment 2

[Toby Elliott [:telliott]]

backend = services.auth.foo.FooClass
interface = services.auth.foo.BaseClass

assuming that BaseClass descends from PluginRegistry, it'll verify that FooClass fulfills the interface before loading the class

Comment 3

[Tarek Ziadé (:tarek)]

oh ok. I was wondering about the split(). You can implement this with resolve_name() and the ABCMeta

Comment 4

[Toby Elliott [:telliott]]

Attachment: Adds the ability to define an interface to load_and_configure

Can't actually build the rpm from this - I seem to have the versioning quirky. But it works up to that point.

Comment 5

[Tarek Ziadé (:tarek)]

Comment on attachment 550552 [details] [diff] [review]
Adds the ability to define an interface to load_and_configure

>diff -r a01dc1f32d1a Makefile
>+from distutils2.util import resolve_name

we don't want our code to depend on distutils2. It's very unstable right now.

and this API exists in our own code in here http://hg.mozilla.org/services/server-core/file/61ba2172b678/services/pluginreg.py#l44

So I propose that we move it now to services.util.resolve_name

> 
> def _resolve_name(name):
>     """Resolves the name and returns the corresponding object."""
>@@ -92,6 +92,11 @@
>     if section:
>         params = filter_params(section, params)
> 
>+    if "interface" in params:

What happens if the name cannot be resolved ? 

if we want to let the error bubble up, which is fine, let's add an example in the tests

Comment 6

[Toby Elliott [:telliott]]

(In reply to comment #5)
> Comment on attachment 550552 [details] [diff] [review] [diff] [details] [review]
> Adds the ability to define an interface to load_and_configure
> 
> >diff -r a01dc1f32d1a Makefile
> >+from distutils2.util import resolve_name
> 
> we don't want our code to depend on distutils2. It's very unstable right now.

Fair enough.

> So I propose that we move it now to services.util.resolve_name
> 

I think it's correct where it is. It's a function that helps with dynamic object loading, and it makes sense to have it with all the other functions that are doing that.

> if we want to let the error bubble up, which is fine, let's add an example
> in the tests

Yep, the error should just bubble up.

Comment 7

[Toby Elliott [:telliott]]

Attachment: version 2 of the loader

Comment 8

[Tarek Ziadé (:tarek)]

Comment on attachment 550734 [details] [diff] [review]
version 2 of the loader

lgtm

Comment 9

[Toby Elliott [:telliott]]

Pushed in http://hg.mozilla.org/services/server-core/rev/de09429e1fee