Closed Bug 674777 Opened 10 years ago Closed 9 years ago

Add support for abc to load_and_configure

Categories

(Cloud Services :: Server: Core, defect)

x86
macOS
defect
Not set
normal

Tracking

(Not tracked)

VERIFIED FIXED

People

(Reporter: telliott, Assigned: telliott)

References

Details

(Whiteboard: [qa-])

Attachments

(1 file, 1 obsolete file)

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.
I am not sure to understand the pseudo-code example, can you elaborate with a configuration file example ?
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
oh ok. I was wondering about the split(). You can implement this with resolve_name() and the ABCMeta
Assignee: nobody → telliott
Can't actually build the rpm from this - I seem to have the versioning quirky. But it works up to that point.
Attachment #550552 - Flags: review?(tarek)
Blocks: 676423
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
Attachment #550552 - Flags: review?(tarek) → review-
(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.
Attachment #550552 - Attachment is obsolete: true
Attachment #550734 - Flags: review?(tarek)
Comment on attachment 550734 [details] [diff] [review]
version 2 of the loader

lgtm
Attachment #550734 - Flags: review?(tarek) → review+
Pushed in http://hg.mozilla.org/services/server-core/rev/de09429e1fee
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Status: RESOLVED → VERIFIED
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.