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