[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