Alon Bar-Lev has posted comments on this change. Change subject: gluster: Added a plugin under gluster for nrpe configuration. ......................................................................
Patch Set 2: (7 comments) http://gerrit.ovirt.org/#/c/26956/2/src/ovirt_host_deploy/constants.py File src/ovirt_host_deploy/constants.py: Line 58: OPENSTACK_NEUTRON_LINUXBRIDGE_CONFIG = \ Line 59: '/etc/neutron/plugins/linuxbridge/linuxbridge_conf.ini' Line 60: OPENSTACK_NEUTRON_OPENVSWITCH_CONFIG = \ Line 61: '/etc/neutron/plugins/openvswitch/ovs_neutron_plugin.ini' Line 62: NRPE_CONFIG_FILE = '/etc/nagios/nrpe.cfg' does nagios supports conf.d structure or only single config? Line 63: Line 64: Line 65: @util.export Line 66: class Defaults(object): Line 118: @util.codegen Line 119: class GlusterEnv(object): Line 120: ENABLE = 'GLUSTER/enable' Line 121: MONITORING_ENABLE = 'GLUSTER/monitoringEnable' Line 122: MONITORING_SERVER = 'GLUSTER/monitoringServer' port is missing, or is syntax is host:port? Line 123: Line 124: Line 125: @util.export Line 126: @util.codegen http://gerrit.ovirt.org/#/c/26956/2/src/plugins/ovirt-host-deploy/gluster/nrpe.py File src/plugins/ovirt-host-deploy/gluster/nrpe.py: Line 41: @plugin.event( Line 42: stage=plugin.Stages.STAGE_INIT, Line 43: ) Line 44: def _setup(self): Line 45: self.environment[odeploycons.GlusterEnv.MONITORING_ENABLE] = False this should be setdefault also please setdefault for the host and port to None Line 46: Line 47: @plugin.event( Line 48: stage=plugin.Stages.STAGE_MISC, Line 49: condition=lambda self: ( Line 48: stage=plugin.Stages.STAGE_MISC, Line 49: condition=lambda self: ( Line 50: self.environment[ Line 51: odeploycons.GlusterEnv.MONITORING_ENABLE Line 52: ] and os.path.isfile(odeploycons.FileLocations.NRPE_CONFIG_FILE) please indent each statement within own block self.environment[ odeploycons.GlusterEnv.MONITORING_ENABLE ] and os.path.isfile(odeploycons.FileLocations.NRPE_CONFIG_FILE) also, I am unsure regarding the isfile, why do you care if it is a file, pipe, symlink? Line 53: ), Line 54: ) Line 55: def _misc(self): Line 56: with open(odeploycons.FileLocations.NRPE_CONFIG_FILE, "r") as f: Line 52: ] and os.path.isfile(odeploycons.FileLocations.NRPE_CONFIG_FILE) Line 53: ), Line 54: ) Line 55: def _misc(self): Line 56: with open(odeploycons.FileLocations.NRPE_CONFIG_FILE, "r") as f: please use single quote Line 57: content = f.readlines() Line 58: content = [ Line 59: (line[:-1] + "," + self.environment[ Line 60: odeploycons.GlusterEnv.MONITORING_SERVER Line 53: ), Line 54: ) Line 55: def _misc(self): Line 56: with open(odeploycons.FileLocations.NRPE_CONFIG_FILE, "r") as f: Line 57: content = f.readlines() please use f.read().splitlines() it is safer. Line 58: content = [ Line 59: (line[:-1] + "," + self.environment[ Line 60: odeploycons.GlusterEnv.MONITORING_SERVER Line 61: ]) Line 60: odeploycons.GlusterEnv.MONITORING_SERVER Line 61: ]) Line 62: if line.startswith("allowed_hosts") Line 63: else line[:-1] for line in content Line 64: ] the above is waaaay to complex for me to understand... :) please use simple loop I understand you try to append? have you checked what happens if: allowed_hosts = , host ? also, running this again and again will append the host every time... which is not wise. Line 65: self.environment[ Line 66: otopicons.CoreEnv.MAIN_TRANSACTION Line 67: ].append( Line 68: filetransaction.FileTransaction( -- To view, visit http://gerrit.ovirt.org/26956 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I4faa40e2bf382187907db944f7485c1630fcc0ae Gerrit-PatchSet: 2 Gerrit-Project: ovirt-host-deploy Gerrit-Branch: master Gerrit-Owner: Darshan N <dnara...@redhat.com> Gerrit-Reviewer: Alon Bar-Lev <alo...@redhat.com> Gerrit-Reviewer: Darshan N <dnara...@redhat.com> Gerrit-Reviewer: Kanagaraj M <kmayi...@redhat.com> Gerrit-Reviewer: Sahina Bose <sab...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
