[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