Merge lp:~soren/vmbuilder/0.12.plugin-priority into lp:vmbuilder

[Soren Hansen]

[Andreas Heck (aheck)]

I think it might be a good idea to define a fixed set of named priority constants and make them type-safe.

[Soren Hansen (soren)]

On Fri, Jan 22, 2010 at 11:28:10PM -0000, Andreas Heck wrote:
> I think it might be a good idea to define a fixed set of named priority constants and make them type-safe.

I'm not sure I see the value in this?

--
Soren Hansen |
Lead virtualisation engineer | Ubuntu Server Team
Canonical Ltd. | http://www.ubuntu.com/

[Andreas Heck (aheck)]

Maybe I misunderstand the purpose of this change but I think the plugin writer should have an easy way to find out which is the right priority for his plugin. So if the plugin writer must read the code of all other existing plugins and find out that his plugin should be best loaded before plugin A and after plugin B than that might be bad encapsulation because you have to look into all plugins to figure out the right prio for your plugin.

It might be better to have some constants with descriptive names like PRIO_CORE, PRIO_DISTRO, PRIO_HYPERVISOR or whatever makes sense. But I don't know enough of the kind of hooks you have in mind so I'm not sure if this can be applied here.

[Soren Hansen (soren)]

The Distro class, for instance, will override the default value of the priority attribute, setting it to 5. It's difficult to anticipate what sort of plugins are going to be created, so it's hard to define much of this in advance. At the moment, my primary concern is to make sure there's an (albeit crude) mechanic for adjusting the order in which plugin's hook functions are run. Once we have more data, we can add more rigour to this structure.

[Andreas Heck (aheck)]

Right, there are some things you just can't know in advance.

review: Approve

=== modified file 'VMBuilder/__init__.py'
--- VMBuilder/__init__.py	2009-12-08 22:27:46 +0000
+++ VMBuilder/__init__.py	2010-01-22 15:31:14 +0000
@@ -52,6 +52,7 @@
 def register_plugin(cls):
     """Register a plugin with VMBuilder"""
     _plugins.append(cls)
+    _plugins.sort(key=lambda x: x.priority)
 
 def set_frontend(arg):
     global frontend

=== modified file 'VMBuilder/plugins/__init__.py'
--- VMBuilder/plugins/__init__.py	2009-06-10 13:40:41 +0000
+++ VMBuilder/plugins/__init__.py	2010-01-22 15:31:14 +0000
@@ -34,6 +34,8 @@
     return retval
 
 class Plugin(object):
+    priority = 10
+
     def __init__(self, vm):
         self.vm = vm
         self.register_options()

=== modified file 'test/util_tests.py'
--- test/util_tests.py	2009-09-12 19:01:53 +0000
+++ test/util_tests.py	2010-01-22 15:31:14 +0000
@@ -7,3 +7,25 @@
     def test_run_cmd(self):
         self.assertTrue("foobarbaztest" in run_cmd("env", env={'foobarbaztest' : 'bar' }))
 
+    def test_plugin_priority(self):
+        class Plugin(object):
+            priority = 10
+
+        class PluginA(Plugin):
+            pass
+
+        class PluginB(Plugin):
+            priority = 5
+
+        class PluginC(Plugin):
+            priority = 15
+
+        saved_plugins = VMBuilder._plugins
+        VMBuilder._plugins = []
+        VMBuilder.register_plugin(PluginA)
+        VMBuilder.register_plugin(PluginB)
+        VMBuilder.register_plugin(PluginC)
+        self.assertEqual(VMBuilder._plugins[0], PluginB)
+        self.assertEqual(VMBuilder._plugins[1], PluginA)
+        self.assertEqual(VMBuilder._plugins[2], PluginC)
+        VMBuilder._plugins = saved_plugins