Sandro Bonazzola has posted comments on this change. Change subject: packaging: setup: Introduce plugin for fence_kdump listener ......................................................................
Patch Set 7: Code-Review-1 (6 comments) See inline comments http://gerrit.ovirt.org/#/c/27486/7/packaging/setup/plugins/ovirt-engine-common/base/core/fence_kdump_listener.py File packaging/setup/plugins/ovirt-engine-common/base/core/fence_kdump_listener.py: Line 19: """fence_kdump listener plugin.""" Line 20: Line 21: Line 22: import gettext Line 23: _ = lambda m: gettext.dgettext(message=m, domain='ovirt-engine-setup') please move call to dgettext after import section in all files. Line 24: Line 25: Line 26: from otopi import util Line 27: from otopi import plugin Line 50: self.logger.info(_('Stopping ovirt-fence-kdump-listener service')) Line 51: self.services.state( Line 52: name=osetupcons.Const.FENCE_KDUMP_LISTENER_SERVICE_NAME, Line 53: state=False Line 54: ) No need to restore the service at closeup? Maybe a comment here may help. Line 55: Line 56: http://gerrit.ovirt.org/#/c/27486/7/packaging/setup/plugins/ovirt-engine-remove/ovirt-engine/system/fence_kdump_listener.py File packaging/setup/plugins/ovirt-engine-remove/ovirt-engine/system/fence_kdump_listener.py: Line 43: def _misc(self): Line 44: self.services.startup( Line 45: name=osetupcons.Const.FENCE_KDUMP_LISTENER_SERVICE_NAME, Line 46: state=False, Line 47: ) isn't this already stoppend at transaction begin by packaging/setup/plugins/ovirt-engine-common/base/core/fence_kdump_listener.py ? Line 48: Line 49: http://gerrit.ovirt.org/#/c/27486/7/packaging/setup/plugins/ovirt-engine-setup/ovirt-engine/fence_kdump_listener/__init__.py File packaging/setup/plugins/ovirt-engine-setup/ovirt-engine/fence_kdump_listener/__init__.py: Line 1: # Line 2: # ovirt-engine-setup -- ovirt engine setup Line 3: # Copyright (C) 2013 Red Hat, Inc. 2014 Line 4: # Line 5: # Licensed under the Apache License, Version 2.0 (the "License"); Line 6: # you may not use this file except in compliance with the License. Line 7: # You may obtain a copy of the License at Line 15: # limitations under the License. Line 16: # Line 17: Line 18: Line 19: """ovirt-host-setup websocket_proxy plugin.""" websocket_proxy? Line 20: Line 21: Line 22: from otopi import util Line 23: http://gerrit.ovirt.org/#/c/27486/7/packaging/setup/plugins/ovirt-engine-setup/ovirt-engine/fence_kdump_listener/config.py File packaging/setup/plugins/ovirt-engine-setup/ovirt-engine/fence_kdump_listener/config.py: Line 99: ) Line 100: self.services.startup( Line 101: name=osetupcons.Const.FENCE_KDUMP_LISTENER_SERVICE_NAME, Line 102: state=True, Line 103: ) if we're handling the service configuration, enable and start here, why do we need to stop it in 2 different plugins? packaging/setup/plugins/ovirt-engine-remove/ovirt-engine/system/fence_kdump_listener.py packaging/setup/plugins/ovirt-engine-common/base/core/fence_kdump_listener.py Line 104: Line 105: -- To view, visit http://gerrit.ovirt.org/27486 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I052cc2296c479caf58d0930a8e678298dde63517 Gerrit-PatchSet: 7 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Peřina <[email protected]> Gerrit-Reviewer: Alon Bar-Lev <[email protected]> Gerrit-Reviewer: Eli Mesika <[email protected]> Gerrit-Reviewer: Lev Veyde <[email protected]> Gerrit-Reviewer: Martin Peřina <[email protected]> Gerrit-Reviewer: Oved Ourfali <[email protected]> Gerrit-Reviewer: Sandro Bonazzola <[email protected]> Gerrit-Reviewer: Simone Tiraboschi <[email protected]> Gerrit-Reviewer: Yedidyah Bar David <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
