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.
asked Nov 1, 2015 at 17:57
\$\endgroup\$
2
  • \$\begingroup\$ What is the purpose of this class, and how do you use it? Optionally, why do you use it? \$\endgroup\$ Commented Nov 2, 2015 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\$ Commented Nov 2, 2015 at 19:37

1 Answer 1

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.

answered Dec 4, 2015 at 23:50
\$\endgroup\$
3
  • 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\$ Commented Dec 5, 2015 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\$ Commented Dec 5, 2015 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\$ Commented Dec 5, 2015 at 0:19

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.