[Yum-devel] single line config files for plugins

[Michael E Brown]

On Mon, May 07, 2007 at 03:14:21PM -0400, Jeremy Katz wrote:
> On Mon, 2007-05-07 at 11:59 -0500, Michael E Brown wrote:
> > On Mon, May 07, 2007 at 11:56:00AM -0400, James Bowes wrote:
> > > Most plugins come with a config file that only includes 'enabled = 1',
> > > why bother making them ship this config file?
> > > 
> > > Maybe instead we should just enable any plugin that doesn't have a
> > > config file, preferably doing it in a way that breaks API?
> > 
> > Is it safe to import the modules without running them? If so, we could
> > easily just look for a variable in the module namespace instead.
> > Something like "YUM_PLUGIN_API_CONFORMS = ...". 
> > 
> > Then, look for a variable in the module to populate default
> > configuration options from. (config file could still be present in this
> > case, it would override options.) I suppose you could even just skip the
> > API check and just look for a config variable. PLUGIN_DEFAULT_CONF =
> > ..., maybe?
> > 
> > Except for the plugin part, this is similar to how we've done mock. We
> > define defaults for all the config options in the code. Then the config
> > file can override any options.
> 
> It should be pretty straight-forward to get plugins to load
> automatically...

Couple points:
    0) The patch as posted doesnt appear to be able to work, as the
    module hasnt been imported yet.
    
    1) I see only "requires_api_version" variable. It makes sense to me
    that we also have a "conforms_to_api_version" variable for this
    case. For example, my dellsysidplugin only requires api version 2.1,
    but complies with a much newer version so that it is both backwards
    and forwards compatible.

    2) we could generalize this more. Why not let the module give us
    it's own config object?  Since sending untested patches seems to be
    in-vogue, how about the attached? Much more generalized.
--
Michael

> 
> Basically, we'd need to change plugins.py:_loadplugin() so that it
> defaults to continuing rather than saying the plugin is disabled.  We
> could then also add a check for a 'default_enabled' attr in the module
> if we wanted.  Something like the following completely and utterly
> untested patch.   
> 
> Could even go one step further and make a new plugin API version add the
> "on by default"-ness and then check the plugin version.  Which probably
> makes some sense.  In any case, it doesn't feel like 3.2.0 material to
> me.
> 
> Jeremy


> _______________________________________________
> Yum-devel mailing list
> Yum-devel at linux.duke.edu
> https://lists.dulug.duke.edu/mailman/listinfo/yum-devel

-------------- next part --------------
--- plugins.py	2007-05-07 14:52:28.000000000 -0500
+++ plugins.py-meb	2007-05-07 14:55:40.000000000 -0500
@@ -176,18 +176,18 @@
         dir, modname = os.path.split(modulefile)
         modname = modname.split('.py')[0]
 
-        conf = self._getpluginconf(modname)
-        if not conf or not config.getOption(conf, 'main', 'enabled', 
-                config.BoolOption(False)):
-            self.verbose_logger.debug('"%s" plugin is disabled', modname)
-            return
-
         fp, pathname, description = imp.find_module(modname, [dir])
         try:
             module = imp.load_module(modname, fp, pathname, description)
         finally:
             fp.close()
 
+        conf = self._getpluginconf(modname, module)
+        if not conf or not config.getOption(conf, 'main', 'enabled', 
+                config.BoolOption(False)):
+            self.verbose_logger.debug('"%s" plugin is disabled', modname)
+            return
+
         # Check API version required by the plugin
         if not hasattr(module, 'requires_api_version'):
              raise Errors.ConfigError(
@@ -245,18 +245,21 @@
                 # Found configuration file
                 break
             self.verbose_logger.log(logginglevels.INFO_2, "Configuration file %s not found" % conffilename)
-        else: # for
-            # Configuration files for the plugin not found
-            self.verbose_logger.log(logginglevels.INFO_2, "Unable to find configuration file for plugin %s"
-                % modname)
-            return None
-        parser = ConfigParser.ConfigParser()
-        confpp_obj = ConfigPreProcessor(conffilename)
-        try:
-            parser.readfp(confpp_obj)
-        except ParsingError, e:
-            raise Errors.ConfigError("Couldn't parse %s: %s" % (conffilename,
-                str(e)))
+
+        parser = None
+        if hasattr(modname, module_default_config):
+            parser = module.module_default_config
+        else:
+            parser = ConfigParser.ConfigParser()
+            confpp_obj = ConfigPreProcessor(conffilename)
+
+        if parser is not None:
+            try:
+                parser.readfp(confpp_obj)
+            except ParsingError, e:
+                raise Errors.ConfigError("Couldn't parse %s: %s" % (conffilename,
+                    str(e)))
+
         return parser
 
     def setCmdLine(self, opts, commands):