Fabian Deutsch has posted comments on this change. Change subject: snmp auto：fix snmp page still shown disabled issue ......................................................................
Patch Set 11: Code-Review-1 (3 comments) Hey, you addressed the previous comments, but also added so many new changes that i can not merge it. Please pull out i.e. the kdump changes into a separate patch. Also please remove the unrelated RHN changes. Also see my inline comments. :) http://gerrit.ovirt.org/#/c/22727/11/scripts/ovirt-auto-install.py File scripts/ovirt-auto-install.py: Line 161: class ConfigureKdump(Transaction.Element): Line 162: title = "Configuring KDump" Line 163: Line 164: def commit(self): Line 165: try: why do you change kdump stuff? Line 166: model = defaults.KDump() Line 167: Line 168: if "OVIRT_KDUMP_SSH" in OVIRT_VARS and \ Line 169: "OVIRT_KDUMP_SSH_KEY" in OVIRT_VARS: http://gerrit.ovirt.org/#/c/22727/11/src/Makefile.am File src/Makefile.am: Line 202: ovirt/node/setup/puppet/puppet_page.py Line 203: Line 204: Line 205: # Setup RHN Plugin Line 206: pyovirt_node_setup_rhn_PYTHON = \ Why is the rhn stuff in here? Where is the hooks.py file? Line 207: ovirt/node/setup/rhn/__init__.py \ Line 208: ovirt/node/setup/rhn/rhn_model.py \ Line 209: ovirt/node/setup/rhn/rhn_page.py Line 210: http://gerrit.ovirt.org/#/c/22727/11/src/ovirtnode/ovirtfunctions.py File src/ovirtnode/ovirtfunctions.py: Line 1200: hookscript = os.path.join(hookdir, hook) Line 1201: if hook.endswith(".py"): Line 1202: pass Line 1203: else: Line 1204: system_closefds(hookscript + " &> /dev/null") You should be using the hook class you created elsewhere. That prevents code duplication. And introduces a single point of responsibility. Line 1205: for f in ["/etc/ssh/ssh_host%s_key" % t for t in ["", "_dsa", "_rsa"]]: Line 1206: ovirt_store_config(f) Line 1207: ovirt_store_config("%s.pub" % f) Line 1208: for f in OVIRT_CONFIG_FILES: -- To view, visit http://gerrit.ovirt.org/22727 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I113f9835d9ac76deaab50bd9e8dfcb034328e441 Gerrit-PatchSet: 11 Gerrit-Project: ovirt-node Gerrit-Branch: master Gerrit-Owner: hadong <[email protected]> Gerrit-Reviewer: Fabian Deutsch <[email protected]> Gerrit-Reviewer: Ryan Barry <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ node-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/node-patches
