Make the loading of extensions to the engine configurable

+1

I added a branch with the general workflow I foresee, any opinions (test/resonance-engine-extension-test.py
 has an example)
Code still needs some work.

There is quite a lot of whitespace twiddling in your branch, making it hard to review...

I can not see exactly how you plan to load the extensions and how you plan to use the __public_methods__ of the extensions..?

Looks good, but:
 - Get ride of the whitespace changes.
 - Please avoid using two underscores (__) for variable names, let's keep those for in-build Python stuff.

How will you get those methods into the D-Bus?

I think that extensions should expose their own DBus APIs. The engine could expose its server-object to allow extensions to do this cleanly...

Regarding the two underscores; the gobject bindings use such constructs as well (for __properties__ and __signals__). So if we have a *really* good reason to keep it like this then I think we could do it. Otherwise we should have something more clean (eg. a simple method or class property).

Sorry, just fixed the whitespace issue to keeps the diff as small as possible; $SOMEONE told my editor to use tabs for indention in all zeitgeist branches, the fact that we are using spaces in some parts of the source code is maybe another bug.
I also renamed __public_methods__ to PUBLIC_METHODS.
We can also register DBus methods dynamically but I don't think we have real use for it right now, let's keep it as simple as possible for now.

I've also added a doctest for the extension mechanism at 'test/test-engine-extension.rst', this file also demos more clearly how this all works. Is it more comprehensible?

sorry, I wanted to link the demo/doctest directly here
http://bazaar.launchpad.net/~thekorn/zeitgeist/zeitgeist-resonance_extensions/annotate/head%3A/test/test-engine-extension.rst

For me there is only one question left to fix this bug: How should methods of extensions be accessible?

Right now they behave like direct members of the engine:
   engine.some_extension_method()

There are also some possible alternative solutions:
   engine.extensions.some_extension_method()
   engine.extensions.NameOfExtensionClass.some_extension_method()

As this is only internal stuff, I think we are safe to go with the current implementation and change over to another one if necessary (e.g. when we get a huge number of extension methods)

IMO this is the only open question we should discuss before merging this branch into lp:zeitgeist.
What do you think?

I think I like the engine.extensions.some_extension_method() approach the best.

This way alternate extensions can provide the same method and the caller does not need to know who implements it.

I believe Markus' branch was merged and the focus stuff removed for now? So we can close this bug?

I don't think we reached the configurable state yet! we should look into it
though for 0.3.1 so for now we can dismiss it

2009/11/26 Mikkel Kamstrup Erlandsen <email address hidden>

> I believe Markus' branch was merged and the focus stuff removed for now?
> So we can close this bug?
>
> --
> [lp:~zeitgeist/zeitgeist/zeitgeist-resonance] make the loading of
> extensions to the engine configurable
> https://bugs.launchpad.net/bugs/483556
> You received this bug notification because you are subscribed to The
> Zeitgeist Project.
>
> Status in Zeitgeist Framework: In Progress
>
> Bug description:
> right now the relevancy-provider is loaded by default. Unfortunately this
> extension is kind of buggy and not well tested, so in case of errors while
> loading this extension the whole engine would fail to run.
> We need a configurable plugin-system for our extensions, and for now we
> should per default disable loading the relevancy provider. If this provider
> gets more stable we should easily be able to add this extension to the
> default extensions which are automatically loaded on engine initialization.
>
>

I just linked lp:~kamstrup/zeitgeist/blacklist to this report because it contains some initial work needed to load extensions dynamically. The work is not nearly done, nor tested or documented, so please hold your reviews back a bit until I ping you :-)

@thekorn: What is the status on this bug report! can we load/unload dynamically now or not?

We can load/unload dynamically to some degree now. Since I merged the blacklist branch we load extensions defined as a list of strings inside _zeitgeist.engine.__init__.py. The final piece of the puzzle IMHO is to allow the user to override this list via an env. var.

Loading and unloading of extensions during a session is not worth the effort I think and also adds considerable DBus book keeping in order to ensure that we close extensions with DBus interfaces cleanly. So only loading from a pre-defined set at startup if you ask me.

what about having the extensions listed in a json file :)

2010/2/16 Mikkel Kamstrup Erlandsen <email address hidden>

> We can load/unload dynamically to some degree now. Since I merged the
> blacklist branch we load extensions defined as a list of strings inside
> _zeitgeist.engine.__init__.py. The final piece of the puzzle IMHO is to
> allow the user to override this list via an env. var.
>
> Loading and unloading of extensions during a session is not worth the
> effort I think and also adds considerable DBus book keeping in order to
> ensure that we close extensions with DBus interfaces cleanly. So only
> loading from a pre-defined set at startup if you ask me.
>
> --
> [lp:~zeitgeist/zeitgeist/zeitgeist-resonance] make the loading of
> extensions to the engine configurable
> https://bugs.launchpad.net/bugs/483556
> You received this bug notification because you are subscribed to The
> Zeitgeist Project.
>
> Status in Zeitgeist Framework: In Progress
>
> Bug description:
> right now the relevancy-provider is loaded by default. Unfortunately this
> extension is kind of buggy and not well tested, so in case of errors while
> loading this extension the whole engine would fail to run.
> We need a configurable plugin-system for our extensions, and for now we
> should per default disable loading the relevancy provider. If this provider
> gets more stable we should easily be able to add this extension to the
> default extensions which are automatically loaded on engine initialization.
>
>
>
>

I don't think there's anyone working on this right now?

I think adding two environment variables would do the trick. We don't want to depend on GConf. When GSettings are available via Python we might want to use those, but envvars are a good start imho. So:

ZEITGEIST_DEFAULT_EXTENSIONS=override the variable _zeitgeist.engine.constants.DEFAULT_EXTENSIONS

ZEITGEIST_EXTRA_EXTENSIONS=extra extensions to load on top of the ones specified by _zeitgeist.engine.constants.DEFAULT_EXTENSIONS

The format of these variables should just be a no-space comma separated list of module.class names line we already have in _zeitgeist.engine.constants.DEFAULT_EXTENSIONS. Fx adding the following lines to my .bashrc would enable the frobnicator- and Tracker event miner extensions:

ZEITGEIST_EXTRA_EXTENSIONS=frobnicator.zg_extensions.ZGFrobicatorExtension,tracker.zg.EventMinerExtension

Depending on GSettings would set a new dependency... Do we want that?
I still think a seperate XML or JSoN file sounds more reasonable.. But again I am not the technical genius so please correct me here... AFAIK GSettings is a GNOME thing. We need to stay platform independent.

Technical note: GSettings is a part of GLib, specifically GIO, and will not as such incur a new dep., except requireing a very recent GLib binding. However I am not sure we'll see bindings before we migrate to an introspection-backed Python (PyGI).

But overall my opinion is simply that we must minimize the number of files to parse or stat during startup.

OK I withdraw my statement

On Tue, May 25, 2010 at 6:27 PM, Mikkel Kamstrup Erlandsen
<email address hidden> wrote:
> Technical note: GSettings is a part of GLib, specifically GIO, and will
> not as such incur a new dep., except requireing a very recent GLib
> binding. However I am not sure we'll see bindings before we migrate to
> an introspection-backed Python (PyGI).
>
> But overall my opinion is simply that we must minimize the number of
> files to parse or stat during startup.
>
> --
> Make the loading of extensions to the engine configurable
> https://bugs.launchpad.net/bugs/483556
> You received this bug notification because you are a bug assignee.
>
> Status in Zeitgeist Framework: Confirmed
>
> Bug description:
> right now the relevancy-provider is loaded by default. Unfortunately this extension is kind of buggy and not well tested, so in case of errors while loading this extension the whole engine would fail to run.
> We need a configurable plugin-system for our extensions, and for now we should per default disable loading the relevancy provider. If this provider gets more stable we should easily be able to add this extension to the default extensions which are automatically loaded on engine initialization.
>
>
>
>

--
This is me doing some advertisement for my blog http://seilo.geekyogre.com

I implemented the behaviour described by Mikkel in comment #17 in revno 1478 of lp:zeitgeist

------------------------------------------------------------
revno: 1478
fixes bug(s): https://launchpad.net/bugs/483556
committer: Markus Korn <email address hidden>
branch nick: trunk
timestamp: Tue 2010-05-25 22:14:11 +0200
message:
  Added two new environment variables `ZEITGEIST_DEFAULT_EXTENSIONs` and
  `ZEITGEIST_EXTRA_EXTENSIONS` to define which extensions are loaded on daemon
  startup. (LP: #483556)
------------------------------------------------------------