Closed Bug 674777 Opened 10 years ago Closed 9 years ago
Add support for abc to load
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
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)
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.
Comment on attachment 550734 [details] [diff] [review] version 2 of the loader lgtm
Attachment #550734 - Flags: review?(tarek) → review+
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.