[PATCH 2/3] Use one thread for payload setup.

David Shea dshea at redhat.com
Fri Aug 22 21:19:34 UTC 2014


Add an object to send events for payload states so that we can have one
copy of the package payload setup code instead of four. This removes
THREAD_PAYLOAD_MD and some duplicated code in THREAD_SOFTWARE_WATCHER,
which also removes the problems of THREAD_PAYLOAD_MD and
THREAD_SOFTWARE_WATCHER potentially accessing the yum environments and
groups properties simultaneously and causing a very confusing crash.

In the payload classes, remove the updateBaseRepo and gatherRepoMetadata
calls from setup so that the remainder of setup can be run separately
from these other two methods. For classes that no longer had any code in
setup, remove the method.
---
 anaconda                                 |   6 +-
 pyanaconda/constants.py                  |   2 +-
 pyanaconda/packaging/__init__.py         | 202 +++++++++++++++++++++++++++++--
 pyanaconda/packaging/dnfpayload.py       |   6 -
 pyanaconda/packaging/rpmostreepayload.py |   3 -
 pyanaconda/packaging/yumpayload.py       |   7 --
 pyanaconda/ui/gui/spokes/network.py      |  12 +-
 pyanaconda/ui/gui/spokes/software.py     |  72 +++++------
 pyanaconda/ui/gui/spokes/source.py       |  93 ++++++--------
 pyanaconda/ui/gui/spokes/storage.py      |   1 -
 pyanaconda/ui/tui/spokes/software.py     |  30 +----
 pyanaconda/ui/tui/spokes/source.py       |  40 ++----
 12 files changed, 285 insertions(+), 189 deletions(-)

diff --git a/anaconda b/anaconda
index 1cffff7..40c201c 100755
--- a/anaconda
+++ b/anaconda
@@ -1306,7 +1306,7 @@ if __name__ == "__main__":
     atexit.register(exitHandler, ksdata.reboot, anaconda.storage)
 
     from blivet import storageInitialize
-    from pyanaconda.packaging import payloadInitialize
+    from pyanaconda.packaging import payloadMgr
     from pyanaconda.network import networkInitialize, wait_for_connecting_NM_thread
     from pyanaconda.timezone import time_initialize
 
@@ -1323,8 +1323,8 @@ if __name__ == "__main__":
                                      args=(ksdata.timezone, anaconda.storage, anaconda.bootloader)))
 
     threadMgr.add(AnacondaThread(name=constants.THREAD_WAIT_FOR_CONNECTING_NM, target=wait_for_connecting_NM_thread, args=(ksdata,)))
-    threadMgr.add(AnacondaThread(name=constants.THREAD_PAYLOAD, target=payloadInitialize,
-                  args=(anaconda.storage, ksdata, anaconda.payload, anaconda.instClass)))
+    payloadMgr.restartThread(anaconda.storage, ksdata, anaconda.payload, anaconda.instClass,
+            fallback=not flags.automatedInstall)
 
     # check if geolocation should be enabled for this type of installation
     use_geolocation = True
diff --git a/pyanaconda/constants.py b/pyanaconda/constants.py
index ae3a750..6981b76 100644
--- a/pyanaconda/constants.py
+++ b/pyanaconda/constants.py
@@ -90,7 +90,7 @@ THREAD_CHECK_STORAGE = "AnaCheckStorageThread"
 THREAD_CUSTOM_STORAGE_INIT = "AnaCustomStorageInit"
 THREAD_WAIT_FOR_CONNECTING_NM = "AnaWaitForConnectingNMThread"
 THREAD_PAYLOAD = "AnaPayloadThread"
-THREAD_PAYLOAD_MD = "AnaPayloadMDThread"
+THREAD_PAYLOAD_RESTART = "AnaPayloadRestartThread"
 THREAD_INPUT_BASENAME = "AnaInputThread"
 THREAD_SYNC_TIME_BASENAME = "AnaSyncTime"
 THREAD_EXCEPTION_HANDLING_TEST = "AnaExceptionHandlingTest"
diff --git a/pyanaconda/packaging/__init__.py b/pyanaconda/packaging/__init__.py
index dcf85fe..b7e66ca 100644
--- a/pyanaconda/packaging/__init__.py
+++ b/pyanaconda/packaging/__init__.py
@@ -35,15 +35,18 @@ import ConfigParser
 import shutil
 import time
 from glob import glob
+import threading
 
 if __name__ == "__main__":
     from pyanaconda import anaconda_log
     anaconda_log.init()
 
 from pyanaconda.constants import DRACUT_ISODIR, DRACUT_REPODIR, DD_ALL, DD_FIRMWARE, DD_RPMS, INSTALL_TREE, ISO_DIR
-from pyanaconda.constants import THREAD_STORAGE, THREAD_WAIT_FOR_CONNECTING_NM
+from pyanaconda.constants import THREAD_STORAGE, THREAD_WAIT_FOR_CONNECTING_NM, THREAD_PAYLOAD
+from pyanaconda.constants import THREAD_PAYLOAD_RESTART
 from pykickstart.constants import GROUP_ALL, GROUP_DEFAULT, GROUP_REQUIRED
 from pyanaconda.flags import flags
+from pyanaconda.i18n import _, N_
 
 from pyanaconda import iutil
 from pyanaconda import isys
@@ -51,6 +54,7 @@ from pyanaconda.image import findFirstIsoImage
 from pyanaconda.image import mountImage
 from pyanaconda.image import opticalInstallMedia
 from pyanaconda.iutil import ProxyString, ProxyStringError
+from pyanaconda.threads import threadMgr, AnacondaThread
 
 from pykickstart.parser import Group
 
@@ -1088,16 +1092,198 @@ class PackagePayload(Payload):
     def groupDescription(self, groupid):
         raise NotImplementedError()
 
-def payloadInitialize(storage, ksdata, payload, instClass):
-    from pyanaconda.threads import threadMgr
+class PayloadManager(object):
+    """Framework for starting and watching the payload thread.
 
-    threadMgr.wait(THREAD_STORAGE)
+       This class defines several states, and events can be triggered upon
+       reaching a state. Depending on whether a state has already been reached
+       when a listener is added, the event code may be run in either the
+       calling thread or the payload thread. The event code will block the
+       payload thread regardless, so try not to run anything that takes a long
+       time.
 
-    # FIXME: condition for cases where we don't want network
-    # (set and use payload.needsNetwork ?)
-    threadMgr.wait(THREAD_WAIT_FOR_CONNECTING_NM)
+       All states except STATE_ERROR are expected to happen linearly, and adding
+       a listener for a state that has already been reached or passed will
+       immediately trigger that listener. For example, if the payload thread is
+       currently in STATE_GROUP_MD, adding a listener for STATE_NETWORK will
+       immediately run the code being added for STATE_NETWORK.
 
-    payload.setup(storage, instClass)
+       The payload thread data should be accessed using the payloadMgr object,
+       and the running thread can be accessed using threadMgr with the
+       THREAD_PAYLOAD constant, if you need to wait for it or something. The
+       thread should be started using payloadMgr.restartThread.
+    """
+
+    STATE_START = 0
+    # Waiting on storage
+    STATE_STORAGE = 1
+    # Waiting on network
+    STATE_NETWORK = 2
+    # Downloading package metadata
+    STATE_PACKAGE_MD = 3
+    # Downloading group metadata
+    STATE_GROUP_MD = 4
+    # All done
+    STATE_FINISHED = 5
+
+    # Error
+    STATE_ERROR = -1
+
+    # Error strings
+    ERROR_SETUP = N_("Failed to set up installation source")
+    ERROR_MD = N_("Error downloading package metadata")
+    ERROR_SOURCE = N_("No installation source avaialble")
+
+    def __init__(self):
+        self._event_lock = threading.Lock()
+        self._event_listeners = {}
+        self._thread_state = self.STATE_START
+        self._error = None
+
+        # Initialize a list for each event state
+        for event_id in range(self.STATE_ERROR, self.STATE_FINISHED + 1):
+            self._event_listeners[event_id] = []
+
+    @property
+    def error(self):
+        return _(self._error)
+
+    def addListener(self, event_id, func):
+        """Add a listener for an event.
+
+           :param int event_id: The event to listen for, one of the EVENT_* constants
+           :param function func: An object to call when the event is reached
+        """
+
+        # Check that the event_id is valid
+        assert isinstance(event_id, int)
+        assert event_id <= self.STATE_FINISHED
+        assert event_id >= self.STATE_ERROR
+
+        # Add the listener inside the lock in case we need to run immediately,
+        # to make sure the listener isn't triggered twice
+        with self._event_lock:
+            self._event_listeners[event_id].append(func)
+
+            # If an error event was requested, run it if currently in an error state
+            if event_id == self.STATE_ERROR:
+                if event_id == self._thread_state:
+                    func()
+            # Otherwise, run if the requested event has already occurred
+            elif event_id <= self._thread_state:
+                func()
+
+    def restartThread(self, storage, ksdata, payload, instClass, fallback=False, checkmount=True):
+        """Start or restart the payload thread.
+
+           If a payload thread is already running, this method will block
+           until the previous thread finishes. This method doesn't lock on
+           anything, so don't try to start two at once.
+
+           :param blivet.Blivet storage: The blivet storage instance
+           :param kickstart.AnacondaKSHandler ksdata: The kickstart data instance
+           :param packaging.Payload payload: The payload instance
+           :param installclass.BaseInstallClass instClass: The install class instance
+           :param bool fallback: Whether to fall back to the default repo in case of error
+           :param bool checkmount: Whether to check for valid mounted media
+        """
+
+        log.debug("Restarting payload thread")
+
+        # If a restart thread is already running, don't start a new one
+        if threadMgr.get(THREAD_PAYLOAD_RESTART):
+            return
+
+        # Launch a new thread so that this method can return immediately
+        threadMgr.add(AnacondaThread(name=THREAD_PAYLOAD_RESTART, target=self._restartThread,
+            args=(storage, ksdata, payload, instClass, fallback, checkmount)))
+
+    def _restartThread(self, storage, ksdata, payload, instClass, fallback, checkmount):
+        # Wait for the old thread to finish
+        threadMgr.wait(THREAD_PAYLOAD)
+
+        # Start a new payload thread
+        threadMgr.add(AnacondaThread(name=THREAD_PAYLOAD, target=self._runThread,
+            args=(storage, ksdata, payload, instClass, fallback, checkmount)))
+
+    def _setState(self, event_id):
+        # Update the current state
+        log.debug("Updating payload thread state: %d", event_id)
+        with self._event_lock:
+            # Update the state within the lock to avoid a race with listeners
+            # currently being added
+            self._thread_state = event_id
+
+            # Run any listeners for the new state
+            for func in self._event_listeners[event_id]:
+                func()
+
+    def _runThread(self, storage, ksdata, payload, instClass, fallback, checkmount):
+        # This is the thread entry
+        # Set the initial state
+        self._error = None
+        self._setState(self.STATE_START)
+
+        # Wait for storage
+        self._setState(self.STATE_STORAGE)
+        threadMgr.wait(THREAD_STORAGE)
+
+        # Wait for network
+        self._setState(self.STATE_NETWORK)
+        # FIXME: condition for cases where we don't want network
+        # (set and use payload.needsNetwork ?)
+        threadMgr.wait(THREAD_WAIT_FOR_CONNECTING_NM)
+
+        self._setState(self.STATE_PACKAGE_MD)
+        payload.setup(storage, instClass)
+
+        # If this is a non-package Payload, we're done
+        if not isinstance(payload, PackagePayload):
+            self._setState(self.STATE_FINISHED)
+            return
+
+        # Keep setting up package-based repositories
+        # Download package metadata
+        try:
+            payload.updateBaseRepo(fallback=fallback, checkmount=checkmount)
+        except (OSError, PayloadError) as e:
+            log.error("PayloadError: %s", e)
+            self._error = self.ERROR_SETUP
+            self._setState(self.STATE_ERROR)
+            return
+
+        # Gather the group data
+        self._setState(self.STATE_GROUP_MD)
+        payload.gatherRepoMetadata()
+        payload.release()
+
+        # Check if that failed
+        if not payload.baseRepo:
+            log.error("No base repo configured")
+            self._error = self.ERROR_MD
+            self._setState(self.STATE_ERROR)
+            return
+
+        try:
+            # Grabbing the list of groups could potentially take a long time the
+            # first time (yum does a lot of magic property stuff, some of which
+            # involves side effects like network access) so go ahead and grab
+            # them now. These are properties with side-effects, just accessing
+            # them will trigger yum.
+            # pylint: disable=pointless-statement
+            payload.environments
+            # pylint: disable=pointless-statement
+            payload.groups
+        except MetadataError as e:
+            log.error("MetadataError: %s", e)
+            self._error = self.ERROR_SOURCE
+            self._setState(self.STATE_ERROR)
+            return
+
+        self._setState(self.STATE_FINISHED)
+
+# Initialize the PayloadManager instance
+payloadMgr = PayloadManager()
 
 def show_groups(payload):
     #repo = ksdata.RepoData(name="anaconda", baseurl="http://cannonball/install/rawhide/os/")
diff --git a/pyanaconda/packaging/dnfpayload.py b/pyanaconda/packaging/dnfpayload.py
index 1814060..70cdff5 100644
--- a/pyanaconda/packaging/dnfpayload.py
+++ b/pyanaconda/packaging/dnfpayload.py
@@ -581,12 +581,6 @@ class DNFPayload(packaging.PackagePayload):
         env = self._base.comps.environment_by_pattern(environmentid)
         map(self.selectGroup, (id_.name for id_ in env.group_ids))
 
-    def setup(self, storage, instClass):
-        # must end up with the base repo (and its metadata) ready
-        super(DNFPayload, self).setup(storage, instClass)
-        self.updateBaseRepo()
-        self.gatherRepoMetadata()
-
     def updateBaseRepo(self, fallback=True, root=None, checkmount=True):
         log.info('configuring base repo')
         self.reset()
diff --git a/pyanaconda/packaging/rpmostreepayload.py b/pyanaconda/packaging/rpmostreepayload.py
index 5c024f1..1dca94a 100644
--- a/pyanaconda/packaging/rpmostreepayload.py
+++ b/pyanaconda/packaging/rpmostreepayload.py
@@ -42,9 +42,6 @@ class RPMOSTreePayload(ArchivePayload):
     def __init__(self, data):
         super(RPMOSTreePayload, self).__init__(data)
 
-    def setup(self, storage, instClass):
-        super(RPMOSTreePayload, self).setup(storage, instClass)
-
     @property
     def handlesBootloaderConfiguration(self):
         return True
diff --git a/pyanaconda/packaging/yumpayload.py b/pyanaconda/packaging/yumpayload.py
index e35db38..4ae61c0 100644
--- a/pyanaconda/packaging/yumpayload.py
+++ b/pyanaconda/packaging/yumpayload.py
@@ -198,13 +198,6 @@ class YumPayload(PackagePayload):
         self._writeYumConfig()
         self._setup = True
 
-        self.updateBaseRepo(fallback=not flags.automatedInstall)
-
-        # When setup is called, it's already in a separate thread. That thread
-        # will try to select groups right after this returns, so make sure we
-        # have group info ready.
-        self.gatherRepoMetadata()
-
     def _resetYum(self, root=None, keep_cache=False, releasever=None):
         """ Delete and recreate the payload's YumBase instance.
 
diff --git a/pyanaconda/ui/gui/spokes/network.py b/pyanaconda/ui/gui/spokes/network.py
index 0c6cce5..b24ddab 100644
--- a/pyanaconda/ui/gui/spokes/network.py
+++ b/pyanaconda/ui/gui/spokes/network.py
@@ -33,7 +33,7 @@ from gi.repository import Gtk
 
 from pyanaconda.flags import can_touch_runtime_system
 from pyanaconda.i18n import _, N_, C_, CN_
-from pyanaconda import constants
+from pyanaconda.flags import flags as anaconda_flags
 from pyanaconda.ui.communication import hubQ
 from pyanaconda.ui.gui import GUIObject
 from pyanaconda.ui.gui.spokes import NormalSpoke, StandaloneSpoke
@@ -1417,13 +1417,9 @@ class NetworkStandaloneSpoke(StandaloneSpoke):
 
         log.debug("network standalone spoke (apply) payload: %s completed: %s", self.payload.baseRepo, self._now_available)
         if not self.payload.baseRepo and not self._initially_available and self._now_available:
-            from pyanaconda.packaging import payloadInitialize
-            from pyanaconda.threads import threadMgr, AnacondaThread
-
-            threadMgr.wait(constants.THREAD_PAYLOAD)
-
-            threadMgr.add(AnacondaThread(name=constants.THREAD_PAYLOAD, target=payloadInitialize,
-                args=(self.storage, self.data, self.payload, self.instclass)))
+            from pyanaconda.packaging import payloadMgr
+            payloadMgr.restartThread(self.storage, self.data, self.payload, self.instclass,
+                    fallback=not anaconda_flags.automatedInstall)
 
         self.network_control_box.kill_nmce(msg="leaving standalone network spoke")
 
diff --git a/pyanaconda/ui/gui/spokes/software.py b/pyanaconda/ui/gui/spokes/software.py
index 2040bf2..ca48f7b 100644
--- a/pyanaconda/ui/gui/spokes/software.py
+++ b/pyanaconda/ui/gui/spokes/software.py
@@ -23,7 +23,7 @@ from gi.repository import Gtk, Pango
 
 from pyanaconda.flags import flags
 from pyanaconda.i18n import _, C_, CN_
-from pyanaconda.packaging import MetadataError, PackagePayload
+from pyanaconda.packaging import PackagePayload, payloadMgr
 from pyanaconda.threads import threadMgr, AnacondaThread
 from pyanaconda import constants, iutil
 
@@ -80,6 +80,25 @@ class SoftwareSelectionSpoke(NormalSpoke):
         self._origAddons = []
         self._origEnvironment = None
 
+        # Register event listeners to update our status on payload events
+        payloadMgr.addListener(payloadMgr.STATE_PACKAGE_MD, self._downloading_package_md)
+        payloadMgr.addListener(payloadMgr.STATE_GROUP_MD, self._downloading_group_md)
+        payloadMgr.addListener(payloadMgr.STATE_FINISHED, self._payload_finished)
+        payloadMgr.addListener(payloadMgr.STATE_ERROR, self._payload_error)
+
+    # Payload event handlers
+    def _downloading_package_md(self):
+        hubQ.send_message(self.__class__.__name__, _("Downloading package metadata..."))
+
+    def _downloading_group_md(self):
+        hubQ.send_message(self.__class__.__name__, _("Downloading group metadata..."))
+
+    def _payload_finished(self):
+        self.environment = self.data.packages.environment
+
+    def _payload_error(self):
+        hubQ.send_message(self.__class__.__name__, payloadMgr.error)
+
     def _apply(self):
         env = self._get_selected_environment()
         if not env:
@@ -129,7 +148,7 @@ class SoftwareSelectionSpoke(NormalSpoke):
     @property
     def completed(self):
         processingDone = bool(not threadMgr.get(constants.THREAD_CHECK_SOFTWARE) and
-                              not threadMgr.get(constants.THREAD_PAYLOAD_MD) and
+                              not threadMgr.get(constants.THREAD_PAYLOAD) and
                               not self._errorMsgs and self.txid_valid)
 
         # we should always check processingDone before checking the other variables,
@@ -166,7 +185,7 @@ class SoftwareSelectionSpoke(NormalSpoke):
         # repo metadata from the mirror list, or we detected a DVD/CD.
 
         return bool(not threadMgr.get(constants.THREAD_SOFTWARE_WATCHER) and
-                    not threadMgr.get(constants.THREAD_PAYLOAD_MD) and
+                    not threadMgr.get(constants.THREAD_PAYLOAD) and
                     not threadMgr.get(constants.THREAD_CHECK_SOFTWARE) and
                     self.payload.baseRepo is not None)
 
@@ -203,44 +222,19 @@ class SoftwareSelectionSpoke(NormalSpoke):
                       target=self._initialize))
 
     def _initialize(self):
-        hubQ.send_message(self.__class__.__name__, _("Downloading package metadata..."))
-
         threadMgr.wait(constants.THREAD_PAYLOAD)
 
-        hubQ.send_message(self.__class__.__name__, _("Downloading group metadata..."))
-
         self.environment = self.data.packages.environment
 
-        if flags.automatedInstall and self.data.packages.seen:
-            # We don't want to do a full refresh, just join the metadata thread
-            threadMgr.wait(constants.THREAD_PAYLOAD_MD)
-        else:
-            # Grabbing the list of groups could potentially take a long time
-            # at first (yum does a lot of magic property stuff, some of which
-            # involves side effects like network access.  We need to reference
-            # them here, outside of the main thread, to not block the UI.
-            try:
-                # pylint: disable=pointless-statement
-                self.payload.environments
-                # pylint: disable=pointless-statement
-                self.payload.groups
-
-                # Parse the environments and groups into the form we want
-                self._parseEnvironments()
-            except MetadataError:
-                hubQ.send_message(self.__class__.__name__,
-                                  _("No installation source available"))
-                return
-
-            # And then having done all that slow downloading, we need to do the first
-            # refresh of the UI here so there's an environment selected by default.
-            # This happens inside the main thread by necessity.  We can't do anything
-            # that takes any real amount of time, or it'll block the UI from updating.
+        if not flags.automatedInstall or not self.data.packages.seen:
+            # having done all the slow downloading, we need to do the first refresh
+            # of the UI here so there's an environment selected by default.  This
+            # happens inside the main thread by necessity.  We can't do anything
+            # that takes any real amount of time, or it'll block the UI from
+            # updating.
             if not self._first_refresh():
                 return
 
-        self.payload.release()
-
         hubQ.send_ready(self.__class__.__name__, False)
 
         # If packages were provided by an input kickstart file (or some other means),
@@ -255,12 +249,8 @@ class SoftwareSelectionSpoke(NormalSpoke):
 
     @gtk_action_wait
     def _first_refresh(self):
-        try:
-            self.refresh()
-            return True
-        except MetadataError:
-            hubQ.send_message(self.__class__.__name__, _("No installation source available"))
-            return False
+        self.refresh()
+        return True
 
     def _add_row(self, listbox, name, desc, button, clicked):
         row = Gtk.ListBoxRow()
@@ -281,7 +271,7 @@ class SoftwareSelectionSpoke(NormalSpoke):
     def refresh(self):
         NormalSpoke.refresh(self)
 
-        threadMgr.wait(constants.THREAD_PAYLOAD_MD)
+        threadMgr.wait(constants.THREAD_PAYLOAD)
 
         if self.environment not in self.payload.environments:
             self.environment = None
diff --git a/pyanaconda/ui/gui/spokes/source.py b/pyanaconda/ui/gui/spokes/source.py
index 1466696..e56b3c5 100644
--- a/pyanaconda/ui/gui/spokes/source.py
+++ b/pyanaconda/ui/gui/spokes/source.py
@@ -42,7 +42,7 @@ from pyanaconda.ui.gui.utils import fire_gtk_action
 from pyanaconda.iutil import ProxyString, ProxyStringError, cmp_obj_attrs
 from pyanaconda.ui.gui.utils import gtk_call_once, really_hide, really_show, fancy_set_sensitive
 from pyanaconda.threads import threadMgr, AnacondaThread
-from pyanaconda.packaging import PayloadError, MetadataError, PackagePayload
+from pyanaconda.packaging import PackagePayload, payloadMgr
 from pyanaconda.regexes import REPO_NAME_VALID, URL_PARSE, HOSTNAME_PATTERN_WITHOUT_ANCHORS
 from pyanaconda import constants
 
@@ -52,7 +52,6 @@ __all__ = ["SourceSpoke"]
 
 BASEREPO_SETUP_MESSAGE = N_("Setting up installation source...")
 METADATA_DOWNLOAD_MESSAGE = N_("Downloading package metadata...")
-METADATA_ERROR_MESSAGE = N_("Error downloading package metadata...")
 
 # These need to match the IDs in protocolComboBox and repoProtocolComboBox in source.glade.
 PROTOCOL_HTTP = 'http'
@@ -369,7 +368,8 @@ class SourceSpoke(NormalSpoke, GUISpokeInputCheckHandler):
         if flags.askmethod:
             flags.askmethod = False
 
-        threadMgr.add(AnacondaThread(name=constants.THREAD_PAYLOAD_MD, target=self.getRepoMetadata))
+        payloadMgr.restartThread(self.storage, self.data, self.payload, self.instclass,
+                checkmount=False)
         self.clear_info()
 
     def _method_changed(self):
@@ -498,55 +498,6 @@ class SourceSpoke(NormalSpoke, GUISpokeInputCheckHandler):
 
         return True
 
-    def getRepoMetadata(self):
-        hubQ.send_not_ready("SoftwareSelectionSpoke")
-        hubQ.send_not_ready(self.__class__.__name__)
-        hubQ.send_message(self.__class__.__name__, _(BASEREPO_SETUP_MESSAGE))
-        # this sleep is lame, but without it the message above doesn't seem
-        # to get processed by the hub in time, and is never shown.
-        # FIXME this should get removed when we figure out how to ensure
-        # that the message takes effect on the hub before we try to mount
-        # a bad NFS server.
-        time.sleep(1)
-        try:
-            self.payload.updateBaseRepo(fallback=False, checkmount=False)
-        except (OSError, PayloadError) as e:
-            log.error("PayloadError: %s", e)
-            self._error = True
-            hubQ.send_message(self.__class__.__name__, _("Failed to set up installation source"))
-            if not (hasattr(self.data.method, "proxy") and self.data.method.proxy):
-                gtk_call_once(self.set_warning, _("Failed to set up installation source; check the repo url"))
-            else:
-                gtk_call_once(self.set_warning, _("Failed to set up installation source; check the repo url and proxy settings"))
-        else:
-            self._error = False
-            hubQ.send_message(self.__class__.__name__, _(METADATA_DOWNLOAD_MESSAGE))
-            self.payload.gatherRepoMetadata()
-            self.payload.release()
-            if not self.payload.baseRepo:
-                hubQ.send_message(self.__class__.__name__, _(METADATA_ERROR_MESSAGE))
-                hubQ.send_ready(self.__class__.__name__, False)
-                self._error = True
-                gtk_call_once(self.set_warning, _("Failed to set up installation source; check the repo url"))
-            else:
-                try:
-                    # Grabbing the list of groups could potentially take a long time the
-                    # first time (yum does a lot of magic property stuff, some of which
-                    # involves side effects like network access) so go ahead and grab
-                    # them now. These are properties with side-effects, just accessing
-                    # them will trigger yum.
-                    # pylint: disable=pointless-statement
-                    self.payload.environments
-                    # pylint: disable=pointless-statement
-                    self.payload.groups
-                except MetadataError:
-                    hubQ.send_message("SoftwareSelectionSpoke",
-                                      _("No installation source available"))
-                else:
-                    hubQ.send_ready("SoftwareSelectionSpoke", False)
-        finally:
-            hubQ.send_ready(self.__class__.__name__, False)
-
     @property
     def changed(self):
         method_changed = self._method_changed()
@@ -570,7 +521,7 @@ class SourceSpoke(NormalSpoke, GUISpokeInputCheckHandler):
     @property
     def ready(self):
         return (self._ready and
-                not threadMgr.get(constants.THREAD_PAYLOAD_MD) and
+                not threadMgr.get(constants.THREAD_PAYLOAD) and
                 not threadMgr.get(constants.THREAD_SOFTWARE_WATCHER) and
                 not threadMgr.get(constants.THREAD_CHECK_SOFTWARE))
 
@@ -680,13 +631,45 @@ class SourceSpoke(NormalSpoke, GUISpokeInputCheckHandler):
 
         threadMgr.add(AnacondaThread(name=constants.THREAD_SOURCE_WATCHER, target=self._initialize))
 
-    def _initialize(self):
+        # Register listeners for payload events
+        payloadMgr.addListener(payloadMgr.STATE_START, self._payload_refresh)
+        payloadMgr.addListener(payloadMgr.STATE_STORAGE, self._probing_storage)
+        payloadMgr.addListener(payloadMgr.STATE_GROUP_MD, self._downloading_package_md)
+        payloadMgr.addListener(payloadMgr.STATE_FINISHED, self._payload_finished)
+        payloadMgr.addListener(payloadMgr.STATE_ERROR, self._payload_error)
+
+    def _payload_refresh(self):
+        hubQ.send_not_ready("SoftwareSelectionSpoke")
+        hubQ.send_not_ready(self.__class__.__name__)
+        hubQ.send_message(self.__class__.__name__, _(BASEREPO_SETUP_MESSAGE))
+
+        # this sleep is lame, but without it the message above doesn't seem
+        # to get processed by the hub in time, and is never shown.
+        # FIXME this should get removed when we figure out how to ensure
+        # that the message takes effect on the hub before we try to mount
+        # a bad NFS server.
+        time.sleep(1)
+
+    def _probing_storage(self):
         hubQ.send_message(self.__class__.__name__, _("Probing storage..."))
 
-        threadMgr.wait(constants.THREAD_STORAGE)
+    def _downloading_package_md(self):
+        # Reset the error state from previous payloads
+        self._error = False
 
         hubQ.send_message(self.__class__.__name__, _(METADATA_DOWNLOAD_MESSAGE))
 
+    def _payload_finished(self):
+        hubQ.send_ready("SoftwareSelectionSpoke", False)
+        hubQ.send_ready(self.__class__.__name__, False)
+
+    def _payload_error(self):
+        self._error = True
+        hubQ.send_message(self.__class__.__name__, payloadMgr.error)
+        gtk_call_once(self.set_warning, _("Failed to set up installation source; check the repo url"))
+        hubQ.send_ready(self.__class__.__name__, False)
+
+    def _initialize(self):
         threadMgr.wait(constants.THREAD_PAYLOAD)
 
         added = False
diff --git a/pyanaconda/ui/gui/spokes/storage.py b/pyanaconda/ui/gui/spokes/storage.py
index 70409b3..1fdb6a8 100644
--- a/pyanaconda/ui/gui/spokes/storage.py
+++ b/pyanaconda/ui/gui/spokes/storage.py
@@ -125,7 +125,6 @@ class InstallOptionsDialogBase(GUIObject):
     def _software_is_ready(self):
         # FIXME:  Would be nicer to just ask the spoke if it's ready.
         return (not threadMgr.get(constants.THREAD_PAYLOAD) and
-                not threadMgr.get(constants.THREAD_PAYLOAD_MD) and
                 not threadMgr.get(constants.THREAD_SOFTWARE_WATCHER) and
                 not threadMgr.get(constants.THREAD_CHECK_SOFTWARE) and
                 self.payload.baseRepo is not None)
diff --git a/pyanaconda/ui/tui/spokes/software.py b/pyanaconda/ui/tui/spokes/software.py
index 690938f..e8866d3 100644
--- a/pyanaconda/ui/tui/spokes/software.py
+++ b/pyanaconda/ui/tui/spokes/software.py
@@ -24,11 +24,11 @@ from pyanaconda.ui.categories.software import SoftwareCategory
 from pyanaconda.ui.tui.spokes import NormalTUISpoke
 from pyanaconda.ui.tui.simpleline import TextWidget, ColumnWidget, CheckboxWidget
 from pyanaconda.threads import threadMgr, AnacondaThread
-from pyanaconda.packaging import MetadataError, DependencyError, PackagePayload
+from pyanaconda.packaging import DependencyError, PackagePayload
 from pyanaconda.i18n import N_, _
 
-from pyanaconda.constants import THREAD_PAYLOAD, THREAD_PAYLOAD_MD
-from pyanaconda.constants import THREAD_CHECK_SOFTWARE, THREAD_SOFTWARE_WATCHER
+from pyanaconda.constants import THREAD_PAYLOAD
+from pyanaconda.constants import THREAD_CHECK_SOFTWARE
 from pyanaconda.constants_text import INPUT_PROCESSED
 
 __all__ = ["SoftwareSpoke"]
@@ -41,7 +41,6 @@ class SoftwareSpoke(NormalTUISpoke):
 
     def __init__(self, app, data, storage, payload, instclass):
         NormalTUISpoke.__init__(self, app, data, storage, payload, instclass)
-        self._ready = False
         self.errors = []
         self._tx_id = None
         # default to first selection (Gnome) in list of environments
@@ -54,26 +53,6 @@ class SoftwareSpoke(NormalTUISpoke):
         # are we taking values (package list) from a kickstart file?
         self._kickstarted = flags.automatedInstall and self.data.packages.seen
 
-    def initialize(self):
-        NormalTUISpoke.initialize(self)
-        threadMgr.add(AnacondaThread(name=THREAD_SOFTWARE_WATCHER, target=self._initialize))
-
-    def _initialize(self):
-        """ Private initialize. """
-        threadMgr.wait(THREAD_PAYLOAD)
-
-        if self._kickstarted:
-            threadMgr.wait(THREAD_PAYLOAD_MD)
-        else:
-            try:
-                self.payload.environments
-            except MetadataError:
-                self.errors.append(_("No installation source available"))
-                return
-        self.payload.release()
-
-        self._ready = True
-
     @property
     def showable(self):
         return isinstance(self.payload, PackagePayload)
@@ -185,8 +164,7 @@ class SoftwareSpoke(NormalTUISpoke):
     @property
     def ready(self):
         """ If we're ready to move on. """
-        return (not threadMgr.get(THREAD_SOFTWARE_WATCHER) and
-                not threadMgr.get(THREAD_PAYLOAD_MD) and
+        return (not threadMgr.get(THREAD_PAYLOAD) and
                 not threadMgr.get(THREAD_CHECK_SOFTWARE))
 
     def apply(self):
diff --git a/pyanaconda/ui/tui/spokes/source.py b/pyanaconda/ui/tui/spokes/source.py
index 928acd4..10786c6 100644
--- a/pyanaconda/ui/tui/spokes/source.py
+++ b/pyanaconda/ui/tui/spokes/source.py
@@ -25,13 +25,13 @@ 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, PackagePayload
+from pyanaconda.packaging import PackagePayload, payloadMgr
 from pyanaconda.i18n import N_, _
 from pyanaconda.image import opticalInstallMedia, potentialHdisoSources
 from pyanaconda.iutil import DataHolder
 
-from pyanaconda.constants import THREAD_SOURCE_WATCHER, THREAD_SOFTWARE_WATCHER, THREAD_PAYLOAD
-from pyanaconda.constants import THREAD_PAYLOAD_MD, THREAD_STORAGE, THREAD_STORAGE_WATCHER
+from pyanaconda.constants import THREAD_SOURCE_WATCHER, THREAD_PAYLOAD
+from pyanaconda.constants import THREAD_STORAGE_WATCHER
 from pyanaconda.constants import THREAD_CHECK_SOFTWARE, ISO_DIR, DRACUT_ISODIR, DRACUT_REPODIR
 from pyanaconda.constants_text import INPUT_PROCESSED
 
@@ -71,10 +71,10 @@ class SourceSpoke(EditTUISpoke, SourceSwitchHandler):
 
         threadMgr.add(AnacondaThread(name=THREAD_SOURCE_WATCHER,
                                      target=self._initialize))
+        payloadMgr.addListener(payloadMgr.STATE_ERROR, self._payload_error)
 
     def _initialize(self):
         """ Private initialize. """
-        threadMgr.wait(THREAD_STORAGE)
         threadMgr.wait(THREAD_PAYLOAD)
         # If we've previously set up to use a CD/DVD method, the media has
         # already been mounted by payload.setup.  We can't try to mount it
@@ -87,6 +87,9 @@ class SourceSpoke(EditTUISpoke, SourceSwitchHandler):
 
         self._ready = True
 
+    def _payload_error(self):
+        self.errors.append(payloadMgr.error)
+
     def _repo_status(self):
         """ Return a string describing repo url or lack of one. """
         if self.data.method.method == "url":
@@ -137,7 +140,6 @@ class SourceSpoke(EditTUISpoke, SourceSwitchHandler):
         EditTUISpoke.refresh(self, args)
 
         threadMgr.wait(THREAD_PAYLOAD)
-        threadMgr.wait(THREAD_PAYLOAD_MD)
 
         _methods = [_("CD/DVD"), _("local ISO file"), _("Network")]
 
@@ -224,33 +226,11 @@ class SourceSpoke(EditTUISpoke, SourceSwitchHandler):
                 self.app.switch_screen(self, num)
         return INPUT_PROCESSED
 
-    def getRepoMetadata(self):
-        """ Pull down yum repo metadata """
-        try:
-            self.payload.updateBaseRepo(fallback=False, checkmount=False)
-        except (OSError, PayloadError) as err:
-            LOG.error("Error: %s", err)
-            self.errors.append(_("Failed to set up installation source"))
-        else:
-            self.payload.gatherRepoMetadata()
-            self.payload.release()
-            if not self.payload.baseRepo:
-                self.errors.append(_("Error downloading package metadata"))
-            else:
-                try:
-                    # pylint: disable=pointless-statement
-                    self.payload.environments
-                    # pylint: disable=pointless-statement
-                    self.payload.groups
-                except MetadataError:
-                    self.errors.append(_("No installation source available"))
-
     @property
     def ready(self):
         """ Check if the spoke is ready. """
         return (self._ready and
-                not threadMgr.get(THREAD_PAYLOAD_MD) and
-                not threadMgr.get(THREAD_SOFTWARE_WATCHER) and
+                not threadMgr.get(THREAD_PAYLOAD) and
                 not threadMgr.get(THREAD_CHECK_SOFTWARE))
 
     def apply(self):
@@ -260,8 +240,8 @@ class SourceSpoke(EditTUISpoke, SourceSwitchHandler):
         if flags.askmethod:
             flags.askmethod = False
 
-        threadMgr.add(AnacondaThread(name=THREAD_PAYLOAD_MD,
-                                     target=self.getRepoMetadata))
+        payloadMgr.restartThread(self.storage, self.data, self.payload, self.instclass,
+                checkmount=False)
 
 class SpecifyRepoSpoke(EditTUISpoke, SourceSwitchHandler):
     """ Specify the repo URL here if closest mirror not selected. """
-- 
1.9.3



More information about the anaconda-patches mailing list