[node-patches] Change in ovirt-node[master]: Add a diagnostic TUI page to view disk information

fabiand at fedoraproject.org fabiand at fedoraproject.org
Mon Feb 25 08:59:52 UTC 2013


Fabian Deutsch has posted comments on this change.

Change subject: Add a diagnostic TUI page to view disk information
......................................................................


Patch Set 1: I would prefer that you didn't submit this

(13 inline comments)

....................................................
File scripts/tui/src/ovirt/node/setup/diagnostic_page.py
Line 19: # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
Line 20: # MA  02110-1301, USA.  A copy of the GNU General Public License is
Line 21: # also available at http://www.gnu.org/copyleft/gpl.html.
Line 22: """
Line 23: Configure KDump
A short description of this page.

It will be displayed when pydoc is used (pydoc ovirt.node.setup.diagnostics_page or node-doc setup.diagnostics_page)
Line 24: """
Line 25: 
Line 26: from ovirt.node import utils, plugins, ui
Line 27: from ovirt.node.config import defaults


Line 23: Configure KDump
Line 24: """
Line 25: 
Line 26: from ovirt.node import utils, plugins, ui
Line 27: from ovirt.node.config import defaults
defaults is not used in this page and can safely be removed
Line 28: from ovirt.node.plugins import Changeset
Line 29: from ovirt.node.utils import console
Line 30: 
Line 31: 


Line 24: """
Line 25: 
Line 26: from ovirt.node import utils, plugins, ui
Line 27: from ovirt.node.config import defaults
Line 28: from ovirt.node.plugins import Changeset
Changeset is not used in this file, so the import can be safely removed
Line 29: from ovirt.node.utils import console
Line 30: 
Line 31: 
Line 32: class Plugin(plugins.NodePlugin):


Line 25: 
Line 26: from ovirt.node import utils, plugins, ui
Line 27: from ovirt.node.config import defaults
Line 28: from ovirt.node.plugins import Changeset
Line 29: from ovirt.node.utils import console
console is not used in this file and can safely be removed
Line 30: 
Line 31: 
Line 32: class Plugin(plugins.NodePlugin):
Line 33: 


Line 56:               ui.Divider("diagnostic.divider"),
Line 57:               #ui.Label("tools.info", "Tools:"),
Line 58:               ui.Table("diagnostic.tools", "", "Available diagnostics",
Line 59:                        self.__diagnostics(), height=len(self.__diagnostics())),
Line 60:               #ui.Divider("diagnostic.divider"),
You can safely remove the following comment lines.
Line 61:               
Line 62:               #We don't need logfiles for now, but there just in case
Line 63:               #ui.Label("logfiles.info", "Logs:"),
Line 64:               #ui.Table("diagnostic.logfiles", "", "Available Logfiles",


Line 57:               #ui.Label("tools.info", "Tools:"),
Line 58:               ui.Table("diagnostic.tools", "", "Available diagnostics",
Line 59:                        self.__diagnostics(), height=len(self.__diagnostics())),
Line 60:               #ui.Divider("diagnostic.divider"),
Line 61:               
This line contains blank spaces. We try to have no spaces (or space chars) on empty lines.
Line 62:               #We don't need logfiles for now, but there just in case
Line 63:               #ui.Label("logfiles.info", "Logs:"),
Line 64:               #ui.Table("diagnostic.logfiles", "", "Available Logfiles",
Line 65:               #         self.__logfiles(), height=len(self.__logfiles())),


Line 75:     def on_merge(self, changes):
Line 76:         if changes.contains_any(["diagnostic.logfiles","diagnostic.tools"]):
Line 77:             cmds = {}
Line 78:             changed_field = changes.keys()[0]
Line 79:             if changed_field == "diagnostic.logfiles":
You could use 
if "diagnostics.logfiles" in change_field
Line 80:                 cmds = {"node": "cat /var/log/ovirt.log | less",
Line 81:                     "ui": "cat /tmp/ovirt.debug.log | less",
Line 82:                     "messages": "cat /var/log/messages | less",
Line 83:                     "dmesg": "dmesg | less"


Line 77:             cmds = {}
Line 78:             changed_field = changes.keys()[0]
Line 79:             if changed_field == "diagnostic.logfiles":
Line 80:                 cmds = {"node": "cat /var/log/ovirt.log | less",
Line 81:                     "ui": "cat /tmp/ovirt.debug.log | less",
Please fix the indentation of this and the following lines so that "ui" lines up with "node" (in the previous line)
Line 82:                     "messages": "cat /var/log/messages | less",
Line 83:                     "dmesg": "dmesg | less"
Line 84:                     }
Line 85:                 


Line 81:                     "ui": "cat /tmp/ovirt.debug.log | less",
Line 82:                     "messages": "cat /var/log/messages | less",
Line 83:                     "dmesg": "dmesg | less"
Line 84:                     }
Line 85:                 
White spaces
Line 86:             elif changed_field == "diagnostic.tools":
Line 87:                 cmds = {"multipath":"multipath -ll | less",
Line 88:                         "fdisk":"fdisk -l | less",
Line 89:                         "parted":"parted -l | less"}


Line 91:             if cmd:
Line 92:                 with self.application.ui.suspended():
Line 93:                     utils.process.call("reset")
Line 94:                     utils.process.call(cmd)
Line 95:             
White spaces
Line 96:                 
Line 97:     def __logfiles(self):
Line 98:         return [("node", "Node Log"),
Line 99:                 ("ui", "Node UI Debug Log"),


Line 92:                 with self.application.ui.suspended():
Line 93:                     utils.process.call("reset")
Line 94:                     utils.process.call(cmd)
Line 95:             
Line 96:                 
White spaces
Line 97:     def __logfiles(self):
Line 98:         return [("node", "Node Log"),
Line 99:                 ("ui", "Node UI Debug Log"),
Line 100:                 ("dmesg", "dmesg"),


Line 93:                     utils.process.call("reset")
Line 94:                     utils.process.call(cmd)
Line 95:             
Line 96:                 
Line 97:     def __logfiles(self):
I'd rather like to remove this function as long as we don't need it ...
Line 98:         return [("node", "Node Log"),
Line 99:                 ("ui", "Node UI Debug Log"),
Line 100:                 ("dmesg", "dmesg"),
Line 101:                 ("messages", "/var/log/messages")]


Line 98:         return [("node", "Node Log"),
Line 99:                 ("ui", "Node UI Debug Log"),
Line 100:                 ("dmesg", "dmesg"),
Line 101:                 ("messages", "/var/log/messages")]
Line 102:         
White spaces
Line 103:     def __diagnostics(self):
Line 104:         return [("multipath", "multipath -ll"),
Line 105:                 ("fdisk", "fdisk -l"),


--
To view, visit http://gerrit.ovirt.org/12339
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia6055bca2369398e2050561fa9388f3a4dfb8535
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-node
Gerrit-Branch: master
Gerrit-Owner: Ryan Barry <rbarry at redhat.com>
Gerrit-Reviewer: Fabian Deutsch <fabiand at fedoraproject.org>



More information about the node-patches mailing list