Martin Peřina has posted comments on this change. Change subject: node: Determine node version ......................................................................
Patch Set 2: (7 comments) http://gerrit.ovirt.org/#/c/29503/2/src/plugins/ovirt-host-deploy/node/node_ver.py File src/plugins/ovirt-host-deploy/node/node_ver.py: > this can go into the detect plugin Well, I didn't want Line 1: # ovirt-host-deploy -- ovirt host deployer Line 2: # Copyright (C) 2012-2014 Red Hat, Inc. Line 3: # Line 4: # This library is free software; you can redistribute it and/or > this can go into the detect plugin Well, I thought it would be more readable to have this in different plugin, but if you want this in detect, I can move it there. Line 1: # ovirt-host-deploy -- ovirt host deployer Line 2: # Copyright (C) 2012-2014 Red Hat, Inc. Line 3: # Line 4: # This library is free software; you can redistribute it and/or Line 47: def _get_node_version(self, version_file): Line 48: version = None Line 49: try: Line 50: with open(version_file) as f: Line 51: version_str = f.read().strip('\n') > read().splitlines() please Done Line 52: for part in version_str.split(' '): Line 53: if ( Line 54: len(part) > 0 and Line 55: part[0].isdigit() Line 54: len(part) > 0 and Line 55: part[0].isdigit() Line 56: ): Line 57: # 1st part starting with number should be version Line 58: version = part > this is bad. we need to file rfe for node for proper version token, we shou Yes, but we need this for 3.5 and it means now. For next version we can improve. Line 59: break Line 60: except (IOError, OSError, ValueError): Line 61: self.logger.debug( Line 62: 'Error getting node version', Line 56: ): Line 57: # 1st part starting with number should be version Line 58: version = part Line 59: break Line 60: except (IOError, OSError, ValueError): > please drop these... we already discuss that. Done Line 61: self.logger.debug( Line 62: 'Error getting node version', Line 63: exc_info=True, Line 64: ) Line 82: for file in version_files: Line 83: version = self._get_node_version(file) Line 84: if version is not None: Line 85: node_version = version Line 86: break > why so complex? Done Line 87: Line 88: self.logger.debug( Line 89: "Node version: '%s'", Line 90: node_version, Line 88: self.logger.debug( Line 89: "Node version: '%s'", Line 90: node_version, Line 91: ) Line 92: self.environment.setdefault( > it is not default, it is hard detect here Done Line 93: odeploycons.VdsmEnv.OVIRT_NODE_VERSION, Line 94: node_version -- 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: 2 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
