3
\$\begingroup\$

I have written a simple system to load plugins from files in Python 2.7, and I'd like to see if any of it can be improved upon.

vedo.py

from __future__ import absolute_import

import os.path
import inspect

from vedo.event_emitter import EventEmitter
from vedo import VedoPlugin


def _get_module_name(directory, filename):
    return '{0}.{1}'.format(os.path.basename(directory), filename)


def _is_plugin_class(plugin_class):
    return (inspect.isclass(plugin_class) and
            issubclass(plugin_class, VedoPlugin) and
            plugin_class != VedoPlugin)


class Vedo(EventEmitter):
    def __init__(self, plugin_directory):
        self._plugin_directory = plugin_directory
        self._plugins = []

    @property
    def plugin_directory(self):
        return self._plugin_directory

    @property
    def plugins(self):
        return self._plugins

    def load_module(self, module):
        plugin_classes = [c for c in module.__dict__.values() if
                          _is_plugin_class(c)]

        for plugin_class in plugin_classes:
            self.plugins.append(plugin_class(self))

    def load_file(self, name):
        try:
            plugin_module = _get_module_name(self._plugin_directory, name)
            namespace = __import__(plugin_module)
        except ImportError:
            return

        self.load_module(getattr(namespace, name))

Each plugin must inherit from VedoPlugin, and is loaded by the Vedo class.

Here's an example plugin:

plugins/first.py

class MyFirstPlugin(VedoPlugin):
    def __init__(self, vedo):
        super(MyFirstPlugin, self).__init__(vedo)

And a quick example of how to use Vedo:

vedo = Vedo("./plugins")
vedo.load_file("first")

vedo.plugins[0] # This is an instance of `MyFirstPlugin'

Here's some things I'm not too fond of, but I'm not sure what I can do about them:

  • Should the private functions in the top of vedo.py be moved inside the Vedo class?
  • Should load_module be private? It's only really going to be used when loading files.
  • Is there a way to get around the ugliness that is the plugin __init__ function? Every single plugin will need to have that constructor, which I'm not too happy about.
\$\endgroup\$
  • \$\begingroup\$ What is the purpose of this class, and how do you use it? Optionally, why do you use it? \$\endgroup\$ – holroy Nov 2 '15 at 15:49
  • \$\begingroup\$ The purpose of the class is to load plugins into an array in a (relatively safe) way. I've added an example of how I use it (normally the plugins listen for events on Vedo passed in their constructor) when loading plugins. I use it as it allows other people to develop plugins and drop them into the project. \$\endgroup\$ – Jack Wilsdon Nov 2 '15 at 19:37
2
\$\begingroup\$

Regarding your questions:

  • No, I think the functions as they are are fine, not everything needs to be moved into a class.
  • If it's an implementation detail, sure.
  • AFAIK no, but if it's otherwise empty the method doesn't have to be added.

Also:

  • In load_module the loop can be more compact by using a combination of list.extend and filter.

    def load_module(self, module):
        self.plugins.extend(filter(_is_plugin_class,
                                module.__dict__.values()))
    
  • The numbered arguments for str.format could be left out.

Looks good otherwise. If you're really concerned with keeping the internals safe you could also not expose the plugins list directly, but provide a separate accessor instead.

\$\endgroup\$
  • 1
    \$\begingroup\$ By "provide a separate accessor", what do you mean? @property makes the value itself read-only, however you could still modify the array using it's methods. Would it be better to return a copy of the array? \$\endgroup\$ – Jack Wilsdon Dec 5 '15 at 0:14
  • \$\begingroup\$ Depends on whether your users could get hold of the list. In that case maybe have a get_plugin method, implement array access on the class, or yes, return a new list copy every time. \$\endgroup\$ – ferada Dec 5 '15 at 0:18
  • \$\begingroup\$ The issue is that I'm not sure if I should "trust" the installed plugins or try to sandbox them as much as possible. I guess there is no proper way to sandbox them, and plugins can only be installed by the user anyway, so hopefully they know what they are doing. \$\endgroup\$ – Jack Wilsdon Dec 5 '15 at 0:19

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.