Yaniv Bronhaim has posted comments on this change.

Change subject: tool: change configurators from classes to modules.
......................................................................


Patch Set 8: Code-Review+1

(5 comments)

put few tiny comments. looks good to me

http://gerrit.ovirt.org/#/c/34047/8/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:

Line 89:     return getattr(module, 'validate', lambda: True)()
Line 90: 
Line 91: 
Line 92: def _configure(module):
Line 93:     """Prepare this machine to run vdsm."""
s/machine/module
Line 94:     getattr(module, 'configure', lambda: None)()
Line 95: 
Line 96: 
Line 97: def _isconfigured(module):


Line 95: 
Line 96: 
Line 97: def _isconfigured(module):
Line 98:     """Return state of configuration. See configurators/__init__.py
Line 99: 
what the __init__ file tells?
Line 100:     Note: returning NOT_CONFIGURED will cause vdsm to abort during
Line 101:     initialization.
Line 102: 
Line 103:     Note: after configure isconfigured should return MAYBE or YES.


Line 96: 
Line 97: def _isconfigured(module):
Line 98:     """Return state of configuration. See configurators/__init__.py
Line 99: 
Line 100:     Note: returning NOT_CONFIGURED will cause vdsm to abort during
NO
Line 101:     initialization.
Line 102: 
Line 103:     Note: after configure isconfigured should return MAYBE or YES.
Line 104:     """


Line 105:     return getattr(module, 'isconfigured', lambda: NO)()
Line 106: 
Line 107: 
Line 108: def _removeConf(module):
Line 109:     getattr(module, 'removeConf', lambda: None)()
no comment? odd..
Line 110: 
Line 111: 
Line 112: @expose("configure")
Line 113: @requiresRoot


http://gerrit.ovirt.org/#/c/34047/8/lib/vdsm/tool/configurators/multipath.py
File lib/vdsm/tool/configurators/multipath.py:

Line 107: 
Line 108: # If multipathd is up, it will be reloaded after configuration,
Line 109: # or started before vdsm starts, so service should not be stopped
Line 110: # during configuration.
Line 111: services = []
why not () ?
Line 112: 
Line 113: 
Line 114: def configure():
Line 115:     """


-- 
To view, visit http://gerrit.ovirt.org/34047
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Iff5d588cfbbcfe0d5f4a90d624d24f21ef4cc580
Gerrit-PatchSet: 8
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: mooli tayer <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
Gerrit-Reviewer: Dima Kuznetsov <[email protected]>
Gerrit-Reviewer: Nir Soffer <[email protected]>
Gerrit-Reviewer: Saggi Mizrahi <[email protected]>
Gerrit-Reviewer: Yaniv Bronhaim <[email protected]>
Gerrit-Reviewer: Yeela Kaplan <[email protected]>
Gerrit-Reviewer: [email protected]
Gerrit-Reviewer: mooli tayer <[email protected]>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to