Alon Bar-Lev has posted comments on this change. Change subject: node: Determine ovirt-node-plugin-vdsm version ......................................................................
Patch Set 5: (8 comments) http://gerrit.ovirt.org/#/c/29503/5/src/plugins/ovirt-host-deploy/node/detect.py File src/plugins/ovirt-host-deploy/node/detect.py: Line 21: """ovirt-node detection.""" Line 22: Line 23: Line 24: import os Line 25: import os.path no need to import os.path, you can use os.path Line 26: import re Line 27: import glob Line 28: import gettext Line 29: _ = lambda m: gettext.dgettext(message=m, domain='ovirt-host-deploy') Line 54: (?P<key>[a-zA-Z0-9_]+) Line 55: = Line 56: (?P<value>.*) Line 57: $ Line 58: """ please go back indent so that single indent per block will exist, add comma after """ Line 59: ) Line 60: Line 61: def __init__(self, context): Line 62: super(Plugin, self).__init__(context=context) Line 62: super(Plugin, self).__init__(context=context) Line 63: Line 64: def _get_node_plugin_version(self, plugin_name): Line 65: result = None Line 66: version_file = '/etc/default/version.%s' % plugin_name if you accept empty plugin_name you can also return the node version at same cost... something like: '/etc/default/version%s' % ('.%s' % plugin name if plugin_name else '') Line 67: if os.path.isfile(version_file): Line 68: content = {} Line 69: with open(version_file) as f: Line 70: for line in f.read().splitlines(): Line 63: Line 64: def _get_node_plugin_version(self, plugin_name): Line 65: result = None Line 66: version_file = '/etc/default/version.%s' % plugin_name Line 67: if os.path.isfile(version_file): what's wrong with exists? Line 68: content = {} Line 69: with open(version_file) as f: Line 70: for line in f.read().splitlines(): Line 71: m = self._PLUGIN_VER_FILE_RE.match(line) Line 74: Line 75: self.logger.debug( Line 76: "Parsed version file: '%s'", Line 77: content, Line 78: ) no need to print the content of the file Line 79: result = ( Line 80: content.get('VERSION', None), Line 81: content.get('RELEASE', None), Line 82: ) Line 83: Line 84: else: Line 85: self.logger.debug( Line 86: "Version file '%s' not found", Line 87: version_file no need for complex messages. just print result before return Line 88: ) Line 89: Line 90: return result Line 91: Line 113: Line 114: if self.environment[odeploycons.VdsmEnv.OVIRT_NODE]: Line 115: # determine version of vdsm plugin Line 116: self.environment[ Line 117: odeploycons.VdsmEnv.NODE_PLUGIN_VERSION this is NODE_VDSM_PLUGIN_VERSION Line 118: ] = self._get_node_plugin_version( Line 119: 'ovirt-node-plugin-vdsm' Line 120: ) Line 121: self.logger.debug( Line 122: "ovirt-node-plugin-vdsm version: '%s'", Line 123: self.environment[ Line 124: odeploycons.VdsmEnv.NODE_PLUGIN_VERSION Line 125: ], Line 126: ) you do not need to debug environment, it is done automatically by the framework Line 127: Line 128: -- To view, visit http://gerrit.ovirt.org/29503 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: Icfa4fb729cb2db3fba9076a1184d2906cee06868 Gerrit-PatchSet: 5 Gerrit-Project: ovirt-host-deploy Gerrit-Branch: master Gerrit-Owner: Martin Peřina <[email protected]> Gerrit-Reviewer: Alon Bar-Lev <[email protected]> Gerrit-Reviewer: Fabian Deutsch <[email protected]> Gerrit-Reviewer: Martin Peřina <[email protected]> Gerrit-Reviewer: Oved Ourfali <[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
