Martin Peřina has posted comments on this change.

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


Patch Set 4:

(6 comments)

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
Done
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
Done
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 #
Done
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....
I used it instead of file exists check, but if you wish to check if file exists 
instead of try/catch, then OK
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
Done
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
Done
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