Re: [PATCH rhel6/master] Support ignore all/reinit all on the disk reinitialization question (#512011).

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

One small nitpick, once fixed feel free to push.

On 01/19/2010 07:23 PM, Chris Lumens wrote:
It's possible for the user to have a huge number of disks, which could result in
this dialog popping up a huge number of times.  Therefore, it'd be a good idea
to allow the user to apply their same choice to all following times when the
dialog would pop up.

Note that it would be nicer to add some sort of checkbox to MessageWindow to
make this more clear, but that is not exactly trivial to do.
---
  gui.py  |   44 ++++++++++++++++++++++++++++++++++++--------
  text.py |   44 ++++++++++++++++++++++++++++++++++++--------
  2 files changed, 72 insertions(+), 16 deletions(-)

diff --git a/gui.py b/gui.py
index 96cc1d4..21863e6 100755
--- a/gui.py
+++ b/gui.py
@@ -1140,21 +1140,35 @@ class InstallInterface:
              log.info("UI not asking about disk initialization, "
                       "using cached answer: %s" % self._initLabelAnswers[path])
              return self._initLabelAnswers[path]
+        elif "all" in self._initLabelAnswers:
+            log.info("UI not asking about disk initialization, "
+                     "using cached answer: %s" % self._initLabelAnswers["all"])
+            return self._initLabelAnswers["all"]

          rc = self.messageWindow(_("Warning"),
                  _("Error processing drive:\n\n"
                    "%(path)s\n%(size)-0.fMB\n%(description)s\n\n"
                    "This device may need to be reinitialized.\n\n"
-                  "REINITIALIZING WILL CAUSE ALL DATA TO BE LOST!%(details)s")
+                  "REINITIALIZING WILL CAUSE ALL DATA TO BE LOST!\n\n"
+                  "This action may also be applied to all other disks "
+                  "needing reinitialization.%(details)s")
                  % {'path': path, 'size': size,
                     'description': description, 'details': details},
                  type="custom",
-                custom_buttons = [ _("_Ignore drive"),
-                                   _("_Re-initialize drive") ],
+                custom_buttons = [ _("_Ignore"),
+                                   _("Ignore _all"),
+                                   _("_Re-initialize"),
+                                   _("Re-ini_tialize all") ],
                  custom_icon="question")
          if rc == 0:
              pass
-        else:
+        elif rc == 1:
+            path = "all"
+            retVal = False
+        elif rc == 2:
+            retVal = True
+        elif rc == 3:
+            path = "all"
              retVal = True

          self._initLabelAnswers[path] = retVal

I think it would be good to replace the "pass" with "retVal = False",
to make it clearer that retVal is False in that case too, that or
remove the setting of "retVal = False" from the rc == 1 case for
consistency.

@@ -1166,6 +1180,7 @@ class InstallInterface:
      def questionReinitInconsistentLVM(self, pv_names=None, lv_name=None, vg_name=None):

          retVal = False # The less destructive default
+        allSet = frozenset(["all"])

          if not pv_names or (lv_name is None and vg_name is None):
              return retVal
@@ -1179,6 +1194,10 @@ class InstallInterface:
              log.info("UI not asking about disk initialization, "
                       "using cached answer: %s" % self._inconsistentLVMAnswers[key])
              return self._inconsistentLVMAnswers[key]
+        elif allSet in self._inconsistentLVMAnswers:
+            log.info("UI not asking about disk initialization, "
+                     "using cached answer: %s" % self._inconsistentLVMAnswers[allSet])
+            return self._inconsistentLVMAnswers[allSet]

          if vg_name is not None:
              message = "Volume Group %s" % vg_name
@@ -1191,15 +1210,24 @@ class InstallInterface:
                      "There is inconsistent LVM data on %(msg)s.  You can "
                      "reinitialize all related PVs (%(pvs)s) which will erase "
                      "the LVM metadata, or ignore which will preserve the "
-                    "contents.") % na,
+                    "contents.  This action may also be applied to all other "
+                    "PVs with inconsistent metadata.") % na,
                  type="custom",
                  custom_buttons = [ _("_Ignore"),
-                                   _("_Re-initialize") ],
+                                   _("Ignore _all"),
+                                   _("_Re-initialize"),
+                                   _("Re-ini_tialize all") ],
                  custom_icon="question")
          if rc == 0:
              pass
-        else:
-            retVal = True # this means clobber.
+        elif rc == 1:
+            key = allSet
+            retVal = False
+        elif rc == 2:
+            retVal = True
+        elif rc == 3:
+            key = allSet
+            retVal = True

          self._inconsistentLVMAnswers[key] = retVal
          return retVal

Same here.

diff --git a/text.py b/text.py
index c4602cd..54c92f2 100644
--- a/text.py
+++ b/text.py
@@ -471,21 +471,35 @@ class InstallInterface:
              log.info("UI not asking about disk initialization, "
                       "using cached answer: %s" % self._initLabelAnswers[path])
              return self._initLabelAnswers[path]
+        elif "all" in self._initLabelAnswers:
+            log.info("UI not asking about disk initialization, "
+                     "using cached answer: %s" % self._initLabelAnswers["all"])
+            return self._initLabelAnswers["all"]

          rc = self.messageWindow(_("Warning"),
                  _("Error processing drive:\n\n"
                    "%(path)s\n%(size)-0.fMB\n%(description)s\n\n"
                    "This device may need to be reinitialized.\n\n"
-                  "REINITIALIZING WILL CAUSE ALL DATA TO BE LOST!%(details)s")
+                  "REINITIALIZING WILL CAUSE ALL DATA TO BE LOST!\n\n"
+                  "This action may also be applied to all other disks "
+                  "needing reinitialization.%(details)s")
                  % {'path': path, 'size': size,
                     'description': description, 'details': details},
                  type="custom",
-                custom_buttons = [ _("_Ignore drive"),
-                                   _("_Re-initialize drive") ],
+                custom_buttons = [ _("_Ignore"),
+                                   _("Ignore _all"),
+                                   _("_Re-initialize"),
+                                   _("Re-ini_tialize all") ],
                  custom_icon="question")
          if rc == 0:
              pass
-        else:
+        elif rc == 1:
+            path = "all"
+            retVal = False
+        elif rc == 2:
+            retVal = True
+        elif rc == 3:
+            path = "all"
              retVal = True

          self._initLabelAnswers[path] = retVal

and here.

@@ -497,6 +511,7 @@ class InstallInterface:
      def questionReinitInconsistentLVM(self, pv_names=None, lv_name=None, vg_name=None):

          retVal = False # The less destructive default
+        allSet = frozenset(["all"])

          if not pv_names or (lv_name is None and vg_name is None):
              return retVal
@@ -510,6 +525,10 @@ class InstallInterface:
              log.info("UI not asking about disk initialization, "
                       "using cached answer: %s" % self._inconsistentLVMAnswers[key])
              return self._inconsistentLVMAnswers[key]
+        elif allSet in self._inconsistentLVMAnswers:
+            log.info("UI not asking about disk initialization, "
+                     "using cached answer: %s" % self._inconsistentLVMAnswers[allSet])
+            return self._inconsistentLVMAnswers[allSet]

          if vg_name is not None:
              message = "Volume Group %s" % vg_name
@@ -522,15 +541,24 @@ class InstallInterface:
                      "There is inconsistent LVM data on %(msg)s.  You can "
                      "reinitialize all related PVs (%(pvs)s) which will erase "
                      "the LVM metadata, or ignore which will preserve the "
-                    "contents.") % na,
+                    "contents.  This action may also be applied to all other "
+                    "PVs with inconsistent metadata.") % na,
                  type="custom",
                  custom_buttons = [ _("_Ignore"),
-                                   _("_Re-initialize") ],
+                                   _("Ignore _all"),
+                                   _("_Re-initialize"),
+                                   _("Re-ini_tialize all") ],
                  custom_icon="question")
          if rc == 0:
              pass
-        else:
-            retVal = True # this means clobber.
+        elif rc == 1:
+            key = allSet
+            retVal = False
+        elif rc == 2:
+            retVal = True
+        elif rc == 3:
+            key = allSet
+            retval = True

          self._inconsistentLVMAnswers[key] = retVal
          return retVal

and here.

Regards,

Hans

_______________________________________________
Anaconda-devel-list mailing list
Anaconda-devel-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/anaconda-devel-list

[Index of Archives]     [Kickstart]     [Fedora Users]     [Fedora Legacy List]     [Fedora Maintainers]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [Yosemite Photos]     [KDE Users]     [Fedora Tools]
  Powered by Linux