[PATCH] Add HDD ISO support for TUI (#1000327)

Martin Kolman mkolman at redhat.com
Thu Oct 31 13:38:06 UTC 2013


On Wed, 2013-10-30 at 13:06 -0400, Samantha N. Bueno wrote:
> On Tue, Oct 29, 2013 at 11:00:44PM +0100, Martin Kolman wrote:
> > It is possible to:
> > * select a mountable device
> > * select a *.iso from the root folder of the device
> > 
> > Signed-off-by: Martin Kolman <mkolman at redhat.com>
> > ---
> >  pyanaconda/ui/tui/spokes/source.py | 336 ++++++++++++++++++++++++++++++++++---
> >  1 file changed, 316 insertions(+), 20 deletions(-)
> > 
> > diff --git a/pyanaconda/ui/tui/spokes/source.py b/pyanaconda/ui/tui/spokes/source.py
> > index 62241a0..2fe286a 100644
> > --- a/pyanaconda/ui/tui/spokes/source.py
> > +++ b/pyanaconda/ui/tui/spokes/source.py
> > @@ -20,18 +20,24 @@
> >  #
> >  
> >  from pyanaconda.flags import flags
> > -from pyanaconda.ui.tui.spokes import EditTUISpoke
> > +from pyanaconda.ui.tui.spokes import EditTUISpoke, NormalTUISpoke
> >  from pyanaconda.ui.tui.spokes import EditTUISpokeEntry as Entry
> >  from pyanaconda.ui.tui.simpleline import TextWidget, ColumnWidget
> >  from pyanaconda.threads import threadMgr, AnacondaThread
> >  from pyanaconda.packaging import PayloadError, MetadataError
> >  from pyanaconda.i18n import _
> > -from pyanaconda.image import opticalInstallMedia
> > +from pyanaconda.image import opticalInstallMedia, potentialHdisoSources
> >  
> >  from pyanaconda.constants import THREAD_SOURCE_WATCHER, THREAD_SOFTWARE_WATCHER, THREAD_PAYLOAD
> > -from pyanaconda.constants import THREAD_PAYLOAD_MD, THREAD_STORAGE, THREAD_CHECK_SOFTWARE
> > +from pyanaconda.constants import THREAD_PAYLOAD_MD, THREAD_STORAGE, THREAD_STORAGE_WATCHER
> > +from pyanaconda.constants import THREAD_CHECK_SOFTWARE, ISO_DIR, DRACUT_ISODIR
> > +
> > +from blivet.util import get_mount_paths
> >  
> >  import re
> > +import os
> > +import fnmatch
> > +import copy
> >  
> >  import logging
> >  LOG = logging.getLogger("anaconda")
> > @@ -39,7 +45,89 @@ LOG = logging.getLogger("anaconda")
> >  
> >  __all__ = ["SourceSpoke"]
> >  
> > -class SourceSpoke(EditTUISpoke):
> > +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):
> > +        self._device = None
> > +        self._current_iso_path = None
> 
> Since you're doing a number of self.data and self.storage manipulations,
> would it not make sense to pass data and storage as arguments to
> __init__?
> 
>     def __init__(self, data, storage):
>         self._device = None
>         self._current_iso_path = None
>         self.data = data
>         self.storage = storage
Actually, this doesn't seem to work, since some of the (quite many)
classes that are used to construct the TUISpoke set data and storage as
RO properties. So I get:
AttributeError: can't set attribute

So I've changed it to use a differently named class-local variables:

def __init__(self, data, storage):
    self._device = None
    self._current_iso_path = None
    self._data = data
    self._storage = storage

And then use self._storage and self._data in place of self.data and
self.storage in the class. I think that should make Pylint happy. :)

> 
> It seems to be working fine regardless, but pylint sure is yelling about
> that.
> 
> > +
> > +    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)
> > +
> 
> < snip.... >
> 
> > +class SelectISOSpoke(NormalTUISpoke, SourceSwitchHandler):
> > < snip.... >
> > +    def apply(self):
> > +        """ Apply all of our changes. """
> > +
> > +        if self._current_iso_path:
> > +            self.set_source_hdd_iso(self._device, self._current_iso_path)
> > +        else:
> > +            self._no_refresh = True
> 
> This flag does not appear to be getting checked anywhere, so it seems
> pointless.
OK, good catch! :)
> 
> Aside from my comments and Vratislav's, I think this looks good.
> 
> Samantha
OK then, fixing locally & I'll push it once I get the needed ACKs.
> 
> > +        # unmount the device - the (YUM) payload will remount it anyway
> > +        # (if it uses it)
> > +        self._unmount_device()
> > -- 
> > 1.8.3.1
> > 
> > _______________________________________________
> > anaconda-patches mailing list
> > anaconda-patches at lists.fedorahosted.org
> > https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches
> _______________________________________________
> anaconda-patches mailing list
> anaconda-patches at lists.fedorahosted.org
> https://lists.fedorahosted.org/mailman/listinfo/anaconda-patches




More information about the anaconda-patches mailing list