Change in vdsm[master]: vdsm-tool: suppoort dependencies between ModuleConfigure
Alon Bar-Lev
alonbl at redhat.com
Wed Aug 13 14:18:54 UTC 2014
Alon Bar-Lev has posted comments on this change.
Change subject: vdsm-tool: suppoort dependencies between ModuleConfigure
......................................................................
Patch Set 2:
(5 comments)
http://gerrit.ovirt.org/#/c/31423/2/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:
Line 195: for requiredName in requiredNames:
Line 196: if requiredName not in retNames:
Line 197: retNames.add(requiredName)
Line 198: queue.append(requiredName)
Line 199: return retNames
I believe that add dependencies and sort can be at the same function, see bellow, maybe I am wrong
Line 200:
Line 201:
Line 202: def _sort_modules(modulesNames):
Line 203: modulesNames = set(modulesNames)
Line 205: sortedModules = []
Line 206: while modulesNames:
Line 207: foundNext = False
Line 208:
Line 209: for c in modulesNames:
I do not understand this algorithm....
you go module by module and try to obtain the dependencies, while only going forward.
should be something recursive like:
f(modules, level):
if level >= 1000: raise 'cirular'
ret = []
for m in modules:
ret += f(m.dependnecies, level+1) + [m,]
return ret
Line 210: requires = _getConfigurers()[c].getRequires()
Line 211: if not requires or requires.issubset(includedNames):
Line 212: foundNext = True
Line 213: modulesNames.remove(c)
Line 207: foundNext = False
Line 208:
Line 209: for c in modulesNames:
Line 210: requires = _getConfigurers()[c].getRequires()
Line 211: if not requires or requires.issubset(includedNames):
empty set is always subset...
Line 212: foundNext = True
Line 213: modulesNames.remove(c)
Line 214: sortedModules.append(c)
Line 215: includedNames.add(c)
Line 211: if not requires or requires.issubset(includedNames):
Line 212: foundNext = True
Line 213: modulesNames.remove(c)
Line 214: sortedModules.append(c)
Line 215: includedNames.add(c)
well, I would have converted the sortedModules to set every time and drop this includedNames optimization, but your call
Line 216: break
Line 217:
Line 218: if not foundNext:
Line 219: raise RuntimeError("Dependency circle found!")
Line 251: args.modules = _getConfigurers().keys()
Line 252: else:
Line 253: _validateModulesNames(args.modules, parser.format_usage())
Line 254:
Line 255: args.modules = _sort_modules(_add_dependencies(args.modules))
why can't the add_dependencies throw an exception if module is missing? removing the above conditional
Line 256:
Line 257: args.modules = [_getConfigurers()[cName] for cName in args.modules]
Line 258:
--
To view, visit http://gerrit.ovirt.org/31423
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: Ibf63ce9aa3ca8edb82091d09b976e8e23896524e
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: mooli tayer <mtayer at redhat.com>
Gerrit-Reviewer: Alon Bar-Lev <alonbl at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: mooli tayer <mtayer at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes
More information about the vdsm-patches
mailing list