Alon Bar-Lev has posted comments on this change.

Change subject: node: Determine ovirt-node-plugin-vdsm version
......................................................................


Patch Set 4:

(7 comments)

http://gerrit.ovirt.org/#/c/29503/4/src/ovirt_host_deploy/constants.py
File src/ovirt_host_deploy/constants.py:

Line 114:     MANAGEMENT_BRIDGE_NAME = 'VDSM/managementBridgeName'
Line 115:     CHECK_VIRT_HARDWARE = 'VDSM/checkVirtHardware'
Line 116:     OVIRT_NODE = 'VDSM/node'
Line 117:     OVIRT_NODE_HAS_OWN_BRIDGES = 'VDSM/nodeHasOwnBridges'
Line 118:     NODE_PLUGIN_VERSION = 'VDSM/nodePluginVersion'
it is vdsm plugin, not generic one... there may be others

hmmmm.... you put this in vdsm env? this belongs to the node env
Line 119:     CONFIG_OVERRIDE = 'VDSM/configOverride'
Line 120:     DISABLE_NETWORKMANAGER = 'VDSM/disableNetworkManager'
Line 121:     CONFIG_PREFIX = 'VDSM_CONFIG/'
Line 122: 


http://gerrit.ovirt.org/#/c/29503/4/src/plugins/ovirt-host-deploy/node/detect.py
File src/plugins/ovirt-host-deploy/node/detect.py:

Line 45:     """
Line 46:     def __init__(self, context):
Line 47:         super(Plugin, self).__init__(context=context)
Line 48: 
Line 49:     def _get_node_plugin_version(self, version_file):
version_file -> plugin_name and construct file here
Line 50:         content = {}
Line 51:         try:
Line 52:             with open(version_file) as f:
Line 53:                 for line in f.read().splitlines():


Line 48: 
Line 49:     def _get_node_plugin_version(self, version_file):
Line 50:         content = {}
Line 51:         try:
Line 52:             with open(version_file) as f:
only if file exists
Line 53:                 for line in f.read().splitlines():
Line 54:                     (key, value) = line.split('=', 1)
Line 55:                     content[key] = value
Line 56:         except (OSError, IOError):


Line 49:     def _get_node_plugin_version(self, version_file):
Line 50:         content = {}
Line 51:         try:
Line 52:             with open(version_file) as f:
Line 53:                 for line in f.read().splitlines():
please ignore lines begins with #

please ignore empty lines
Line 54:                     (key, value) = line.split('=', 1)
Line 55:                     content[key] = value
Line 56:         except (OSError, IOError):
Line 57:             self.logger.debug(


Line 52:             with open(version_file) as f:
Line 53:                 for line in f.read().splitlines():
Line 54:                     (key, value) = line.split('=', 1)
Line 55:                     content[key] = value
Line 56:         except (OSError, IOError):
please do not use these catch blocks, the usual comment....
Line 57:             self.logger.debug(
Line 58:                 "Version file '%s' not found",
Line 59:                 exc_info=True,
Line 60:             )


Line 60:             )
Line 61:         return (
Line 62:             content.get('VERSION', None),
Line 63:             content.get('RELEASE', None),
Line 64:         )
return should be None if plugin is not installed
Line 65: 
Line 66:     @plugin.event(
Line 67:         stage=plugin.Stages.STAGE_INIT,
Line 68:         priority=plugin.Stages.PRIORITY_FIRST,


Line 99:                 ][0],
Line 100:                 self.environment[
Line 101:                     odeploycons.VdsmEnv.NODE_PLUGIN_VERSION
Line 102:                 ][1],
Line 103:             )
this is debug, just print the tuple
Line 104: 
Line 105: 


-- 
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: 4
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

Reply via email to