[Yum-devel] single line config files for plugins

Michael E Brown Michael_E_Brown at dell.com
Mon May 7 20:00:19 UTC 2007


On Mon, May 07, 2007 at 02:58:38PM -0500, Michael E Brown wrote:
> On Mon, May 07, 2007 at 02:57:02PM -0500, Michael E Brown wrote:
> > 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.

Sorry for the noise. I attached the old version again.
--
Michael

-------------- next part --------------
--- plugins.py	2007-05-07 14:52:28.000000000 -0500
+++ plugins.py-meb	2007-05-07 14:57:45.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(
@@ -234,7 +234,7 @@
                         (modname, getattr(module, funcname))
                         )
 
-    def _getpluginconf(self, modname):
+    def _getpluginconf(self, modname, moduleobj):
         '''Parse the plugin specific configuration file and return a
         IncludingConfigParser instance representing it. Returns None if there
         was an error reading or parsing the configuration file.
@@ -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(moduleobj, module_default_config):
+            parser = moduleobj.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):


More information about the Yum-devel mailing list