[PATCH 16/16] Split ksdata execute and setup methods for addons

Vratislav Podzimek vpodzime at redhat.com
Thu Dec 13 16:45:21 UTC 2012


On Thu, 2012-12-06 at 16:46 +0100, Martin Sivak wrote:
> ---
>  pyanaconda/addons.py          | 47 ++++++++++++++++++++++++++++++++-----------
>  pyanaconda/install.py         |  5 ++++-
>  pyanaconda/kickstart.py       |  2 +-
>  pyanaconda/ui/gui/__init__.py | 10 ++++++---
>  4 files changed, 47 insertions(+), 17 deletions(-)
> 
> diff --git a/pyanaconda/addons.py b/pyanaconda/addons.py
> index 0220107..c2510fa 100644
> --- a/pyanaconda/addons.py
> +++ b/pyanaconda/addons.py
> @@ -23,7 +23,8 @@ __all__ = ["AddonSection", "AddonRegistry", "AddonData", "collect_addon_paths"]
>  
>  import os
>  from pykickstart.sections import Section
> -    
> +from pykickstart.parser import KSOptionParser
> +
>  def collect_addon_paths(toplevel_addon_paths, ui_subdir = "gui"):
>      """This method looks into the directories present
>         in toplevel_addon_paths and registers each subdirectory
> @@ -39,13 +40,13 @@ def collect_addon_paths(toplevel_addon_paths, ui_subdir = "gui"):
>          "ks": [],
>          "categories": []
>          }
> -    
> +
>      for path in toplevel_addon_paths:
>          try:
>              files = os.listdir(path)
>          except OSError:
>              files = []
> -                
> +
>          for addon_id in files:
>              addon_ks_path = os.path.join(path, addon_id, "ks")
>              if os.path.isdir(addon_ks_path):
> @@ -68,7 +69,7 @@ class AddonRegistry(object):
>  
>         It acts as a proxy during kickstart save.
>      """
> -       
> +
>      def __init__(self, dictionary):
>          self.__dict__ = dictionary
>  
> @@ -76,11 +77,20 @@ class AddonRegistry(object):
>          return reduce(lambda acc,(id, addon): acc + "%%addon %s\n%s%%end\n" % (id, str(addon)),
>                        self.__dict__.iteritems(), "")
>  
> -    def execute(self, storage, ksdata, instClass):
> +    # pylint: disable-msg=C0103
> +    def execute(self, storage, ksdata, instClass, users):
>          """This method calls execute on all the registered addons."""
>          for k, v in self.__dict__.iteritems():
> -            v.execute(storage, ksdata, instClass)
> -        
> +            if hasattr(v, "execute"):
> +                v.execute(storage, ksdata, instClass, users)
Isn't execute method mandatory? I guess you are trying to prevent
tracebacks, but we should probably check that much sooner, or provide

def execute(...):
    pass

in the generic Addon class.

> +
> +    def setup(self, storage, ksdata, instClass):
> +        """This method calls setup on all the registered addons."""
> +        for k, v in self.__dict__.iteritems():
> +            if hasattr(v, "setup"):
> +                v.setup(storage, ksdata, instClass)
> +
> +
>  class AddonData(object):
>      """This is a common parent class for loading and storing
>         3rd party data to kickstart. It is instantiated by
> @@ -96,7 +106,7 @@ class AddonData(object):
>         There is also a mandatory method execute, which should
>         make all the described changes to the installed system.
>      """
> -    
> +
>      def __init__(self, name):
>          self.name = name
>          self.content = ""
> @@ -104,8 +114,21 @@ class AddonData(object):
>      def __str__(self):
>          return self.content
>  
> -    def execute(self, storage, ksdata, instClass):
> -        """Make the changes to the underlying system."""
> +    # pylint: disable-msg=C0103
> +    def setup(self, storage, ksdata, instClass):
> +        """Make the changes to the install system.
> +
> +           This method is called before the installation
> +           is started and directly from spokes. It must be possible
> +           to call it multiple times without breaking the environment."""
> +        pass
> +
> +    def execute(self, storage, ksdata, instClass, users):
> +        """Make the changes to the underlying system.
> +
> +           This method is called only once in the post-install
> +           setup phase.
> +        """
>          pass
>  
>      def handle_line(self, line):
> @@ -133,10 +156,10 @@ class AddonSection(Section):
>          """Process the arguments to the %addon header."""
>          Section.handleHeader(self, lineno, args)
>          op = KSOptionParser(version=self.version)
> -        (opts, extra) = op.parse_args(args=args[1:], lineno=lineno)
> +        (_opts, extra) = op.parse_args(args=args[1:], lineno=lineno)
Why this   ^^^^^^ renaming?

>          self.addon_id = extra[0]
>  
>          # if the addon is not registered, create dummy placeholder for it
>          if self.addon_id and not hasattr(self.handler.addon, self.addon_id):
> -            setattr(self.handler.addon, self.addon_id, AnacondaKSAddon(self.addon_id))
> +            setattr(self.handler.addon, self.addon_id, AddonData(self.addon_id))
>  
> diff --git a/pyanaconda/install.py b/pyanaconda/install.py
> index 4e552ea..af214ba 100644
> --- a/pyanaconda/install.py
> +++ b/pyanaconda/install.py
> @@ -109,9 +109,12 @@ def doInstall(storage, payload, ksdata, instClass):
>      steps = len(storage.devicetree.findActions(type="create", object="format")) + \
>              len(storage.devicetree.findActions(type="resize", object="format")) + \
>              len(storage.devicetree.findActions(type="migrate", object="format"))
> -    steps += 4  # packages setup, packages, bootloader, post install
> +    steps += 5  # pre setup phase, packages setup, packages, bootloader, post install
>      progress.send_init(steps)
>  
> +    with progress_report(_("Setting up the install environment")):
> +        ksdata.addon.setup(storage, ksdata, instClass)
> +
>      # Do partitioning.
>      payload.preStorage()
>      turnOnFilesystems(storage)
> diff --git a/pyanaconda/kickstart.py b/pyanaconda/kickstart.py
> index 693ecd4..b242e92 100644
> --- a/pyanaconda/kickstart.py
> +++ b/pyanaconda/kickstart.py
> @@ -1432,7 +1432,7 @@ def preScriptPass(f):
>  def parseKickstart(f):
>      # preprocessing the kickstart file has already been handled in initramfs.
>  
> -    handler = AnacondaKSHandler(constants.addon_paths)
> +    handler = AnacondaKSHandler(addon_paths)
Will this work? Are you importing addon_paths from the constants module
somewhere?

>      ksparser = AnacondaKSParser(handler)
>  
>      # We need this so all the /dev/disk/* stuff is set up before parsing.
> diff --git a/pyanaconda/ui/gui/__init__.py b/pyanaconda/ui/gui/__init__.py
> index 2056307..1ffb728 100644
> --- a/pyanaconda/ui/gui/__init__.py
> +++ b/pyanaconda/ui/gui/__init__.py
> @@ -18,7 +18,7 @@
>  #
>  # Red Hat Author(s): Chris Lumens <clumens at redhat.com>
>  #
> -import importlib, inspect, os, sys, time
> +import inspect, os, sys, time
>  import meh.ui.gui
>  
>  from gi.repository import Gdk
> @@ -120,8 +120,12 @@ class GUIObject(common.UIObject):
>  
>      def _findUIFile(self):
>          path = os.environ.get("UIPATH", "./:/tmp/updates/:/tmp/updates/ui/:/usr/share/anaconda/ui/")
> +
> +        # look for files in the same dir as where the class is defined
> +        path += ":%s" % os.path.dirname(inspect.getfile(self.__class__))
> +        
>          for d in path.split(":"):
> -            testPath = os.path.normpath(d + self.uiFile)
> +            testPath = os.path.normpath(os.path.join(d, self.uiFile))
>              if os.path.isfile(testPath) and os.access(testPath, os.R_OK):
>                  return testPath
>  
> @@ -265,7 +269,7 @@ class GraphicalUserInterface(UserInterface):
>          return [SummaryHub, ProgressHub]
>  
>      def _is_standalone(self, obj):
> -        """Is the spoke passes as obj standalone?"""
> +        """Is the spoke passed as obj standalone?"""
Okay, there is a fix for one of my previous comments.

-- 
Vratislav Podzimek

Anaconda Rider | Red Hat, Inc. | Brno - Czech Republic



More information about the anaconda-patches mailing list