This is only a beginning, but hopefully enough to get the idea across. The purpose of this is twofold: For mixins/interfaces/whatever that do not make sense unless inherited by another class, don't allow the class to be instantiated. For mixins/interfaces/whatever that depend on properties that are implemented in the inheriting class or its other subclasses, make the relationship explicit through the use of abstract properties.
SourceSwitchHandler is a good example, and the catalyst for this. SourceSwitchHandler manipulates self.data, which is provided in the subclass by way of EditTUISpoke -> UIObject. This kind of a relationship is flagged by pylint since .data is not provided by SourceSwitchHandler or any of its subclasses. b5df3a90 attempted to remedy this by storing a copy of data in SourceSwitchHandler, but that commit is incorrect: self.data in SourceSwitchHandler will resolve to UIObject.data when called from a class that inherits from both, and UIObject.data is a read-only property. This commit instead makes SourceSwitchHandler an abstract base class. It implements a data property of its own but requires that the property be overridden by another class. This way no class can inherit from SourceSwitchHandler without providing a data property. --- po/POTFILES.in | 1 + pyanaconda/ui/gui/spokes/storage.py | 36 +------ pyanaconda/ui/interfaces.py | 184 ++++++++++++++++++++++++++++++++++++ pyanaconda/ui/tui/spokes/source.py | 95 ++----------------- 4 files changed, 196 insertions(+), 120 deletions(-) create mode 100644 pyanaconda/ui/interfaces.py
diff --git a/po/POTFILES.in b/po/POTFILES.in index af263f9..c1d3a7e 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -28,6 +28,7 @@ pyanaconda/packaging/yumpayload.py
# Interfaces pyanaconda/ui/common.py +pyanaconda/ui/interfaces.py pyanaconda/ui/__init__.py
# Common stuff diff --git a/pyanaconda/ui/gui/spokes/storage.py b/pyanaconda/ui/gui/spokes/storage.py index 4192a1a..ce80c72 100644 --- a/pyanaconda/ui/gui/spokes/storage.py +++ b/pyanaconda/ui/gui/spokes/storage.py @@ -50,6 +50,7 @@ from pyanaconda.ui.gui.spokes.lib.detailederror import DetailedErrorDialog from pyanaconda.ui.gui.spokes.lib.resize import ResizeDialog from pyanaconda.ui.gui.categories.system import SystemCategory from pyanaconda.ui.gui.utils import enlightbox, escape_markup +from pyanaconda.ui.interfaces import StorageChecker
from pyanaconda.kickstart import doKickstartStorage, getAvailableDiskSpace from blivet import empty_device @@ -306,33 +307,6 @@ class InstallOptions3Dialog(InstallOptions1Dialog): def continue_response(self): return self.RESPONSE_CONTINUE_NONE
-class StorageChecker(object): - errors = [] - warnings = [] - _mainSpokeClass = "StorageSpoke" - - def __init__(self): - # This is provided by the StorageSpoke class, which is a subclass of - # this one. Backwards, I know. - self.storage = None - - def run(self): - threadMgr.add(AnacondaThread(name=constants.THREAD_CHECK_STORAGE, - target=self.checkStorage)) - - def checkStorage(self): - threadMgr.wait(constants.THREAD_EXECUTE_STORAGE) - - hubQ.send_not_ready(self._mainSpokeClass) - hubQ.send_message(self._mainSpokeClass, _("Checking storage configuration...")) - (StorageChecker.errors, - StorageChecker.warnings) = self.storage.sanityCheck() - hubQ.send_ready(self._mainSpokeClass, True) - for e in StorageChecker.errors: - log.error(e) - for w in StorageChecker.warnings: - log.warn(w) - class StorageSpoke(NormalSpoke, StorageChecker): builderObjects = ["storageWindow", "addSpecializedImage"] mainWidgetName = "storageWindow" @@ -431,7 +405,7 @@ class StorageSpoke(NormalSpoke, StorageChecker): doKickstartStorage(self.storage, self.data, self.instclass) except (StorageError, KickstartValueError) as e: log.error("storage configuration failed: %s", e) - StorageChecker.errors = str(e).split("\n") + self.errors = str(e).split("\n") hubQ.send_message(self.__class__.__name__, _("Failed to save storage configuration...")) self.data.bootloader.bootDrive = "" self.data.ignoredisk.drives = [] @@ -443,14 +417,14 @@ class StorageSpoke(NormalSpoke, StorageChecker): self._applyDiskSelection(self.selected_disks) except BootLoaderError as e: log.error("BootLoader setup failed: %s", e) - StorageChecker.errors = str(e).split("\n") + self.errors = str(e).split("\n") hubQ.send_message(self.__class__.__name__, _("Failed to save storage configuration...")) self.data.bootloader.bootDrive = "" else: if self.autopart: # this was already run as part of doAutoPartition. dumb. - StorageChecker.errors = [] - StorageChecker.warnings = [] + self.errors = [] + self.warnings = [] self.run() finally: self._ready = True diff --git a/pyanaconda/ui/interfaces.py b/pyanaconda/ui/interfaces.py new file mode 100644 index 0000000..6b46d03 --- /dev/null +++ b/pyanaconda/ui/interfaces.py @@ -0,0 +1,184 @@ +# Abstract interfaces for UI classes +# +# Copyright (C) 2013 Red Hat, Inc. +# +# This copyrighted material is made available to anyone wishing to use, +# modify, copy, or redistribute it subject to the terms and conditions of +# the GNU General Public License v.2, or (at your option) any later version. +# This program is distributed in the hope that it will be useful, but WITHOUT +# ANY WARRANTY expressed or implied, including the implied warranties of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General +# Public License for more details. You should have received a copy of the +# GNU General Public License along with this program; if not, write to the +# Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA +# 02110-1301, USA. Any Red Hat trademarks that are incorporated in the +# source code or documentation are not subject to the GNU General Public +# License and may only be used or replicated with the express permission of +# Red Hat, Inc. +# +# Red Hat Author(s): David Shea <dshea@redhat.com +# + +# This file contains abstract base classes that provide specific functionality +# that can be added to another class. The idea is sort-of modelled after Java's +# interfaces. An abstract base class cannot be instatiated, and it provides a +# contract for classes that inherit from it: any method or property marked as +# abstract in the base class must be overriden in the inheriting class. This +# allows for cleaner implementation of certain types of mixin-classes: a class +# that adds functionality to another class can explicitly require that methods +# or properties be provided by the inheriting class or another superclass of +# the inheriting class. +# +# In general, classes that inherit from abstract base classes should place the +# abstract base class at the end of the inheritance list. This way any abstract +# methods or properties in the abc will be overridden by the base classes +# that are first in the inheritance list. For example, an abstract base class +# may add a method that reads from spoke.data: +# +# class Mixin(object): +# __metaclass__ = ABCMeta +# +# @abstractproperty +# def data(self): pass +# +# def isHD(self): +# return self.data.method == "harddrive" +# +# The Mixin class will add the method isHD to any class that inherits from it, +# and classes that inherit from Mixin must provide a data property. +# +# class MixedObject(UIObject, Mixin): +# .... +# +# The method resolution order of MixedObject resolves UIObject.data before +# Mixin.data, so UIObject.data satisfies the requirment that Mixin.data be +# overriden. + +from abc import ABCMeta, abstractproperty + +from pyanaconda import constants +from pyanaconda.threads import threadMgr, AnacondaThread +from pyanaconda.ui.communication import hubQ +from pyanaconda.i18n import _ + +import logging +import copy + +class StorageChecker(object): + __metaclass__ = ABCMeta + + log = logging.getLogger("anaconda") + + def __init__(self): + self.errors = [] + self.warnings = [] + + @abstractproperty + def storage(self): pass + + def run(self): + threadMgr.add(AnacondaThread(name=constants.THREAD_CHECK_STORAGE, + target=self.checkStorage)) + + def checkStorage(self): + threadMgr.wait(constants.THREAD_EXECUTE_STORAGE) + + hubQ.send_not_read(self.__class__.__name__) + hubQ.send_message(self.__class__.__name__, _("Check storage configuration...")) + (self.errors, + self.warnings) = self.storage.sanityCheck() + for e in self.errors: + self.log.error(e) + for w in self.warnings: + self.log.warn(w) + +class SourceSwitchHandler(object): + """ A class that can be used as a mixin handling + installation source switching. + It will correctly switch to the new method + and cleanup any previous method set. + """ + + __metaclass__ = ABCMeta + + @abstractproperty + def data(self): pass + + @abstractproperty + def storage(self): pass + + def __init__(self): + self._device = None + self._current_iso_path = None + + def _clean_hdd_iso(self): + """ Clean HDD ISO usage + This means unmounting the partition and unprotecting it, + so it can be used for the installation. + """ + if self.data.method.method == "harddrive" and self.data.method.partition: + part = self.data.method.partition + dev = self.storage.devicetree.getDeviceByName(part) + if dev: + dev.protected = False + self.storage.config.protectedDevSpecs.remove(part) + + def set_source_hdd_iso(self, device, iso_path): + """ Switch to the HDD ISO install source + :param partition: name of the partition hosting the ISO + :type partition: string + :param iso_path: full path to the source ISO file + :type iso_path: string + """ + partition = device.name + # the GUI source spoke also does the copy + old_source = copy.copy(self.data.method) + + # if a different partition was used previously, unprotect it + if old_source.method == "harddrive" and old_source.partition != partition: + self._clean_hdd_iso() + + # protect current device + if device: + device.protected = True + self.storage.config.protectedDevSpecs.append(device.name) + + self.data.method.method = "harddrive" + self.data.method.partition = partition + # the / gets stripped off by payload.ISOImage + self.data.method.dir = "/" + iso_path + + # as we already made the device protected when + # switching to it, we don't need to protect it here + + def set_source_url(self, url=None): + """ Switch to install source specified by URL """ + # clean any old HDD ISO sources + self._clean_hdd_iso() + + self.data.method.method = "url" + if url is not None: + self.data.method.url = url + + def set_source_nfs(self, opts=None): + """ Switch to NFS install source """ + # clean any old HDD ISO sources + self._clean_hdd_iso() + + self.data.method.method = "nfs" + if opts is not None: + self.data.method.opts = opts + + def set_source_cdrom(self): + """ Switch to cdrom install source """ + # clean any old HDD ISO sources + self._clean_hdd_iso() + + self.data.method.method = "cdrom" + + def set_source_closest_mirror(self): + """ Switch to the closest mirror install source """ + # clean any old HDD ISO sources + self._clean_hdd_iso() + + self.data.method.method = None diff --git a/pyanaconda/ui/tui/spokes/source.py b/pyanaconda/ui/tui/spokes/source.py index 977e7df..a3397fd 100644 --- a/pyanaconda/ui/tui/spokes/source.py +++ b/pyanaconda/ui/tui/spokes/source.py @@ -33,12 +33,13 @@ from pyanaconda.constants import THREAD_PAYLOAD_MD, THREAD_STORAGE, THREAD_STORA from pyanaconda.constants import THREAD_CHECK_SOFTWARE, ISO_DIR, DRACUT_ISODIR from pyanaconda.constants_text import INPUT_PROCESSED
+from pyanaconda.ui.interfaces import SourceSwitchHandler + from blivet.util import get_mount_paths
import re import os import fnmatch -import copy
import logging LOG = logging.getLogger("anaconda") @@ -46,90 +47,6 @@ LOG = logging.getLogger("anaconda")
__all__ = ["SourceSpoke"]
-class SourceSwitchHandler(object): - """ A class that can be used as a mixin handling - installation source switching. - It will correctly switch to the new method - and cleanup any previous method set. - """ - def __init__(self, data, storage): - self._device = None - self._current_iso_path = None - self.data = data - self.storage = storage - - def _clean_hdd_iso(self): - """ Clean HDD ISO usage - This means unmounting the partition and unprotecting it, - so it can be used for the installation. - """ - if self.data.method.method == "harddrive" and self.data.method.partition: - part = self.data.method.partition - dev = self.storage.devicetree.getDeviceByName(part) - if dev: - dev.protected = False - self.storage.config.protectedDevSpecs.remove(part) - - def set_source_hdd_iso(self, device, iso_path): - """ Switch to the HDD ISO install source - :param partition: name of the partition hosting the ISO - :type partition: string - :param iso_path: full path to the source ISO file - :type iso_path: string - """ - partition = device.name - # the GUI source spoke also does the copy - old_source = copy.copy(self.data.method) - - # if a different partition was used previously, unprotect it - if old_source.method == "harddrive" and old_source.partition != partition: - self._clean_hdd_iso() - - # protect current device - if device: - device.protected = True - self.storage.config.protectedDevSpecs.append(device.name) - - self.data.method.method = "harddrive" - self.data.method.partition = partition - # the / gets stripped off by payload.ISOImage - self.data.method.dir = "/" + iso_path - - # as we already made the device protected when - # switching to it, we don't need to protect it here - - def set_source_url(self, url=None): - """ Switch to install source specified by URL """ - # clean any old HDD ISO sources - self._clean_hdd_iso() - - self.data.method.method = "url" - if url is not None: - self.data.method.url = url - - def set_source_nfs(self, opts=None): - """ Switch to NFS install source """ - # clean any old HDD ISO sources - self._clean_hdd_iso() - - self.data.method.method = "nfs" - if opts is not None: - self.data.method.opts = opts - - def set_source_cdrom(self): - """ Switch to cdrom install source """ - # clean any old HDD ISO sources - self._clean_hdd_iso() - - self.data.method.method = "cdrom" - - def set_source_closest_mirror(self): - """ Switch to the closest mirror install source """ - # clean any old HDD ISO sources - self._clean_hdd_iso() - - self.data.method.method = None - class SourceSpoke(EditTUISpoke, SourceSwitchHandler): """ Spoke used to customize the install source repo. """ title = N_("Installation source") @@ -142,7 +59,7 @@ class SourceSpoke(EditTUISpoke, SourceSwitchHandler):
def __init__(self, app, data, storage, payload, instclass): EditTUISpoke.__init__(self, app, data, storage, payload, instclass) - SourceSwitchHandler.__init__(self, data, storage) + SourceSwitchHandler.__init__(self) self._ready = False self.errors = [] self._cdrom = None @@ -344,7 +261,7 @@ class SpecifyRepoSpoke(EditTUISpoke, SourceSwitchHandler):
def __init__(self, app, data, storage, payload, instclass, selection): EditTUISpoke.__init__(self, app, data, storage, payload, instclass) - SourceSwitchHandler.__init__(self, data, storage) + SourceSwitchHandler.__init__(self) self.selection = selection self.args = self.data.method
@@ -383,7 +300,7 @@ class SpecifyNFSRepoSpoke(EditTUISpoke, SourceSwitchHandler):
def __init__(self, app, data, storage, payload, instclass, selection, errors): EditTUISpoke.__init__(self, app, data, storage, payload, instclass) - SourceSwitchHandler.__init__(self, data, storage) + SourceSwitchHandler.__init__(self) self.selection = selection self.args = self.data.method self.errors = errors @@ -504,7 +421,7 @@ class SelectISOSpoke(NormalTUISpoke, SourceSwitchHandler):
def __init__(self, app, data, storage, payload, instclass, device): NormalTUISpoke.__init__(self, app, data, storage, payload, instclass) - SourceSwitchHandler.__init__(self, data, storage) + SourceSwitchHandler.__init__(self) self.selection = None self.args = self.data.method self._device = device