[PATCH] Add a Spoke.changed attribute.

Chris Lumens clumens at redhat.com
Wed Apr 10 18:07:06 UTC 2013


Previously, we were checking whether a spoke changed to avoid needlessly
running apply/execute from within those methods on each spoke.  This is
something the hub could do instead, though.

As a practical improvement, this means going into the storage spoke, changing
nothing, and then leaving means it won't attempt to redo storage.  The user
can still outsmart this check, though, but I think for most people it will be
fine.

Note that not all spoke do these checks yet.  I have updated those that do.
---
 pyanaconda/ui/common.py              | 11 ++++++++++-
 pyanaconda/ui/gui/hubs/__init__.py   |  8 ++++----
 pyanaconda/ui/gui/spokes/software.py | 20 +++++++++++++++-----
 pyanaconda/ui/gui/spokes/source.py   | 13 ++++++-------
 pyanaconda/ui/gui/spokes/storage.py  | 19 +++++++++++++++++++
 pyanaconda/ui/tui/hubs/summary.py    |  3 ++-
 6 files changed, 56 insertions(+), 18 deletions(-)

diff --git a/pyanaconda/ui/common.py b/pyanaconda/ui/common.py
index 8248bd5..2bae250 100644
--- a/pyanaconda/ui/common.py
+++ b/pyanaconda/ui/common.py
@@ -232,6 +232,15 @@ class Spoke(UIObject):
         raise NotImplementedError
 
     @property
+    def changed(self):
+        """Have the values on the spoke changed since the last time it was
+           run?  If not, the apply and execute methods will be skipped.  This
+           is to avoid the spoke doing potentially long-lived and destructive
+           actions that are completely unnecessary.
+        """
+        return True
+
+    @property
     def configured(self):
         """This method returns a list of textual ids that should
            be written into the after-install customization status
@@ -257,7 +266,7 @@ class Spoke(UIObject):
            Spokes are mandatory unless marked as not being so.
         """
         return True
-    
+
     def execute(self):
         """Cause the data object to take effect on the target system.  This will
            usually be as simple as calling one or more of the execute methods on
diff --git a/pyanaconda/ui/gui/hubs/__init__.py b/pyanaconda/ui/gui/hubs/__init__.py
index 446f168..53143d4 100644
--- a/pyanaconda/ui/gui/hubs/__init__.py
+++ b/pyanaconda/ui/gui/hubs/__init__.py
@@ -116,7 +116,7 @@ class Hub(GUIObject, common.Hub):
         Gtk.main()
         action.window.set_transient_for(None)
 
-        if not action.skipTo or (action.skipTo and action.applyOnSkip):
+        if action.changed and (not action.skipTo or (action.skipTo and action.applyOnSkip)):
             action.apply()
             action.execute()
 
@@ -128,7 +128,7 @@ class Hub(GUIObject, common.Hub):
         """
 
         ret = {}
-        
+
         # Collect all the categories this hub displays, then collect all the
         # spokes belonging to all those categories.
         categories = sorted(filter(lambda c: c.displayOnHub == self.__class__, collect_categories(self.paths["categories"])),
@@ -206,7 +206,7 @@ class Hub(GUIObject, common.Hub):
                 spoke.selector.connect("key-release-event", self._on_spoke_clicked, spoke)
 
                 # If this is a kickstart install, attempt to execute any provided ksdata now.
-                if flags.automatedInstall and spoke.ready:
+                if flags.automatedInstall and spoke.ready and spoke.changed:
                     spoke.execute()
 
                 selectors.append(spoke.selector)
@@ -327,7 +327,7 @@ class Hub(GUIObject, common.Hub):
                     # _createBox skipped.  Now that it's become ready, do it.  Note
                     # that we also provide a way to skip this processing (see comments
                     # communication.py) to prevent getting caught in a loop.
-                    if not args[1]:
+                    if not args[1] and spoke.changed:
                         spoke.execute()
 
                     if self.continuePossible:
diff --git a/pyanaconda/ui/gui/spokes/software.py b/pyanaconda/ui/gui/spokes/software.py
index e9cf708..55af428 100644
--- a/pyanaconda/ui/gui/spokes/software.py
+++ b/pyanaconda/ui/gui/spokes/software.py
@@ -71,11 +71,6 @@ class SoftwareSelectionSpoke(NormalSpoke):
 
         addons = self._get_selected_addons()
 
-        # Don't redo dep solving if nothing's changed.
-        if row[2] == self._origEnvironment and set(addons) == set(self._origAddons) and \
-           self.txid_valid:
-            return
-
         self._selectFlag = False
         self.payload.data.packages.groupList = []
         self.payload.selectEnvironment(row[2])
@@ -123,6 +118,21 @@ class SoftwareSelectionSpoke(NormalSpoke):
             return self._get_selected_environment() is not None and processingDone
 
     @property
+    def changed(self):
+        row = self._get_selected_environment()
+        if not row:
+            return True
+
+        addons = self._get_selected_addons()
+
+        # Don't redo dep solving if nothing's changed.
+        if row[2] == self._origEnvironment and set(addons) == set(self._origAddons) and \
+           self.txid_valid:
+            return False
+
+        return True
+
+    @property
     def mandatory(self):
         return True
 
diff --git a/pyanaconda/ui/gui/spokes/source.py b/pyanaconda/ui/gui/spokes/source.py
index 1a2b280..accd19a 100644
--- a/pyanaconda/ui/gui/spokes/source.py
+++ b/pyanaconda/ui/gui/spokes/source.py
@@ -278,13 +278,8 @@ class SourceSpoke(NormalSpoke):
         if flags.askmethod:
             flags.askmethod = False
 
-        method_changed = self._method_changed()
-        repos_changed = self._update_payload_repos()
-
-        if method_changed or repos_changed:
-            threadMgr.add(AnacondaThread(name=constants.THREAD_PAYLOAD_MD,
-                                         target=self.getRepoMetadata))
-            self.clear_info()
+        threadMgr.add(AnacondaThread(name=constants.THREAD_PAYLOAD_MD, target=self.getRepoMetadata))
+        self.clear_info()
 
     def _method_changed(self):
         """ Check to see if the install method has changed.
@@ -452,6 +447,10 @@ class SourceSpoke(NormalSpoke):
             hubQ.send_ready(self.__class__.__name__, False)
 
     @property
+    def changed(self):
+        return self._method_changed() or self._update_payload_repos()
+
+    @property
     def completed(self):
         if flags.automatedInstall and (not self.data.method.method or not self.payload.baseRepo):
             return False
diff --git a/pyanaconda/ui/gui/spokes/storage.py b/pyanaconda/ui/gui/spokes/storage.py
index 17634d7..8b6fbaa 100644
--- a/pyanaconda/ui/gui/spokes/storage.py
+++ b/pyanaconda/ui/gui/spokes/storage.py
@@ -406,6 +406,14 @@ class StorageSpoke(NormalSpoke, StorageChecker):
             hubQ.send_ready(self.__class__.__name__, True)
 
     @property
+    def changed(self):
+        disksChanged = set(self._initial_selected_disks) != set(self.selected_disks)
+        bootloaderChanged = self.data.bootloader.bootDrive is None or \
+                            self._initial_bootloader_disk != self.data.bootloader.bootDrive
+
+        return disksChanged or bootloaderChanged
+
+    @property
     def completed(self):
         retval = (threadMgr.get(constants.THREAD_EXECUTE_STORAGE) is None and
                   threadMgr.get(constants.THREAD_CHECK_STORAGE) is None and
@@ -513,6 +521,12 @@ class StorageSpoke(NormalSpoke, StorageChecker):
 
         self._update_summary()
 
+        # This list is used to determine if anything has changed in storage
+        # configuration since the user entered the spoke.  If not, we shouldn't
+        # do anything when Done is clicked.
+        self._initial_selected_disks = self.selected_disks[:]
+        self._initial_bootloader_disk = self.data.bootloader.bootDrive
+
         if self.errors:
             self.set_warning(_("Error checking storage configuration.  Click for details."))
         elif self.warnings:
@@ -696,6 +710,11 @@ class StorageSpoke(NormalSpoke, StorageChecker):
         return True
 
     def on_back_clicked(self, button):
+        # Did the user change anything?  If not, this method is a no-op.
+        if not self.changed:
+            NormalSpoke.on_back_clicked(self, button)
+            return
+
         # Remove all non-existing devices if autopart was active when we last
         # refreshed.
         if self._previous_autopart:
diff --git a/pyanaconda/ui/tui/hubs/summary.py b/pyanaconda/ui/tui/hubs/summary.py
index 965fdc1..ab8bb39 100644
--- a/pyanaconda/ui/tui/hubs/summary.py
+++ b/pyanaconda/ui/tui/hubs/summary.py
@@ -45,7 +45,8 @@ class SummaryHub(TUIHub):
 
             print('')
             for spoke in spokes:
-                spoke.execute()
+                if spoke.changed:
+                    spoke.execute()
 
     # override the prompt so that we can skip user input on kickstarts
     # where all the data is in hand.  If not in hand, do the actual prompt.
-- 
1.8.1.2



More information about the anaconda-patches mailing list